656919 - Add a Buildbot based tinderboxData backend to TBPL

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[Markus Stange [:mstange]]

Attachment: add BuildbotDBUser and move MachineResults to a new JS file

The MachineResult change is pure code moving.

Comment 1

[Markus Stange [:mstange]]

Attachment: add usebuildbot=1 and branch=... url params

Until the kinks with the buildbot backend are worked out, we shouldn't use it by default. This adds a usebuildbot=1 url flag to enable it.

I'm also adding a way to specify the treeName by using the buildbot branch name. In PHP land we don't know the Tinderbox tree name that corresponds to a buildbot branch, but I'd still like to be able to link to TBPL from there. The only place where I'm doing that at the moment is in Short/FullLogGenerator.php: these logs link to the corresponding TBPL push view.

Comment 2

[Arpad Borsos [:Swatinem]]

Comment on attachment 532232 [details] [diff] [review]
add BuildbotDBUser and move MachineResults to a new JS file

Review of attachment 532232 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with nits fixed.

::: js/BuildbotDBUser.js
@@ +15,5 @@
> +        dataType: 'json',
> +        success: function (json) {
> +          var loadedData = {};
> +          loadedData[push.defaultTip] = json.filter(function (build) { return Config.hiddenBuilds.indexOf(build.buildername) == -1; });
> +          loadCallback(self._createMachineResults(tree, data, loadedData));

We make one request per push, so there is no need to create a dict with push.defaultTip as key. That would also simplify _createMachineResults a bit (no more for (var rev in loadedData)), just pass the rev as argument.

@@ +51,5 @@
> +          "reftestLogURL": Config.baseURL + "php/getLogExcerpt.php?id=" + build._id + '&type=reftest',
> +          "tinderboxPrintURL": Config.baseURL + "php/getLogExcerpt.php?id=" + build._id + '&type=tinderbox_print',
> +          "revs": revs,
> +          "notes": [],
> +          "errorParser": 'unittest',

Do we still need this?

@@ +52,5 @@
> +          "tinderboxPrintURL": Config.baseURL + "php/getLogExcerpt.php?id=" + build._id + '&type=tinderbox_print',
> +          "revs": revs,
> +          "notes": [],
> +          "errorParser": 'unittest',
> +          "_getScrape": function (callback) {

MachineResult.prototype is probably a better place for this, also given that you call it from other functions defined on the prototype.

::: js/MachineResult.js
@@ +97,5 @@
> +      };
> +    }).filter(function filterNull() { return this; }).get());
> +  },
> +
> +  getBuildIDForSimilarBuild: function MachineResult_getBuildIDForSimilarBuild(callback, failCallback, timeoutCallback) {

I guess this compat code can go once we use buildbot ids everywhere?

@@ +138,5 @@
> +  },
> +};
> +
> +if (!String.prototype.trim) {
> +  String.prototype.trim = function String_trim() {

Note to self: move this to a better location.

Comment 3

[Arpad Borsos [:Swatinem]]

Comment on attachment 532234 [details] [diff] [review]
add usebuildbot=1 and branch=... url params

Review of attachment 532234 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/Controller.js
@@ +60,5 @@
>      var params = this._parseParams();
> +
> +    // Allow requests using the buildbot branch name instead of the Tinderbox
> +    // tree name, but redirect such requests to the tree name form.
> +    // e.g. redirect ?branch=mozilla-1.9.2 to ?tree=Firefox3.6

We can probably drop the notion of “tree” and just use “branch” everywhere when:
- The bb branch is unique (no more Firefox and Mobile share the same bb branchname)
- Every tree has a bb branch. Which ones are missing right now?

Comment 4

[Arpad Borsos [:Swatinem]]

Comment on attachment 532232 [details] [diff] [review]
add BuildbotDBUser and move MachineResults to a new JS file

Review of attachment 532232 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/MachineResult.js
@@ +138,5 @@
> +  },
> +};
> +
> +if (!String.prototype.trim) {
> +  String.prototype.trim = function String_trim() {

Now that I have cleaned up the other utility functions, make sure to move this to utils.js as well.

Comment 5

[Markus Stange [:mstange]]

Attachment: add BuildbotDBUser and move MachineResults to a new JS file

(In reply to comment #2)
> ::: js/BuildbotDBUser.js
> @@ +15,5 @@
> > +        dataType: 'json',
> > +        success: function (json) {
> > +          var loadedData = {};
> > +          loadedData[push.defaultTip] = json.filter(function (build) { return Config.hiddenBuilds.indexOf(build.buildername) == -1; });
> > +          loadCallback(self._createMachineResults(tree, data, loadedData));
> 
> We make one request per push, so there is no need to create a dict with
> push.defaultTip as key. That would also simplify _createMachineResults a bit
> (no more for (var rev in loadedData)), just pass the rev as argument.

Done.

> > +          "reftestLogURL": Config.baseURL + "php/getLogExcerpt.php?id=" + build._id + '&type=reftest',
> > +          "tinderboxPrintURL": Config.baseURL + "php/getLogExcerpt.php?id=" + build._id + '&type=tinderbox_print',
> > +          "revs": revs,
> > +          "notes": [],
> > +          "errorParser": 'unittest',
> 
> Do we still need this?

Nope, bug 601743 will remove the only user. Good catch.

> 
> @@ +52,5 @@
> > +          "tinderboxPrintURL": Config.baseURL + "php/getLogExcerpt.php?id=" + build._id + '&type=tinderbox_print',
> > +          "revs": revs,
> > +          "notes": [],
> > +          "errorParser": 'unittest',
> > +          "_getScrape": function (callback) {
> 
> MachineResult.prototype is probably a better place for this, also given that
> you call it from other functions defined on the prototype.

Moved.

> ::: js/MachineResult.js
> @@ +97,5 @@
> > +      };
> > +    }).filter(function filterNull() { return this; }).get());
> > +  },
> > +
> > +  getBuildIDForSimilarBuild: function MachineResult_getBuildIDForSimilarBuild(callback, failCallback, timeoutCallback) {
> 
> I guess this compat code can go once we use buildbot ids everywhere?

Good point, I hadn't even looked at this part before. I've short circuited that method by initializing _similarBuildID with the right build id from the start.

> ::: js/MachineResult.js
> @@ +138,5 @@
> > +  },
> > +};
> > +
> > +if (!String.prototype.trim) {
> > +  String.prototype.trim = function String_trim() {
> 
> Now that I have cleaned up the other utility functions, make sure to move
> this to utils.js as well.

Done.