Bug 183194 - BuildbotBuildEntry for buildbot 0.9 uses incorrect buildrequestid
Summary: BuildbotBuildEntry for buildbot 0.9 uses incorrect buildrequestid
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
Depends on:
Blocks: 175969
  Show dependency treegraph
 
Reported: 2018-02-27 19:13 PST by Aakash Jain
Modified: 2018-03-12 17:17 PDT (History)
8 users (show)

See Also:


Attachments
Patch - WIP (5.17 KB, patch)
2018-03-04 11:30 PST, Aakash Jain
no flags Details | Formatted Diff | Diff
Proposed patch (5.37 KB, patch)
2018-03-05 17:51 PST, Aakash Jain
rniwa: review+
Details | Formatted Diff | Diff
Patch for landing (5.62 KB, patch)
2018-03-09 14:50 PST, Aakash Jain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2018-02-27 19:13:53 PST
Buildbot 0.9 has a property named 'buildrequestid'. Also perf syncing script set a build property named 'build-request-id'. Both these properties looks very similar in name but have very different purpose. 

https://trac.webkit.org/browser/webkit/trunk/Websites/perf.webkit.org/tools/js/buildbot-syncer.js#L18 uses 'buildrequestid' provided by buildbot. This is not what we want. We want to fetch the 'build-request-id' set by us in the properties.

We should correct BuildbotBuildEntry to use the right variable. Also, we should also rename 'build-request-id' to something else to avoid confusion in future.
Comment 1 Aakash Jain 2018-02-27 19:15:54 PST
The issue is with pending build-requests. Buildbot API for buildrequests doesn't provide properties. We need to figure out a solution to that first.
Comment 2 Aakash Jain 2018-03-04 10:04:05 PST
> The issue is with pending build-requests. Buildbot API for buildrequests doesn't provide properties. We need to figure out a solution to that first.
Buildbot API now provides the properties for build-requests after the fix in https://github.com/buildbot/buildbot/pull/3976
Comment 3 Aakash Jain 2018-03-04 11:30:57 PST
Created attachment 334982 [details]
Patch - WIP
Comment 4 Aakash Jain 2018-03-05 17:51:53 PST
Created attachment 335060 [details]
Proposed patch

Tested using unit-test. Will test by running syncing scripts as well. Please review meanwhile.
Comment 5 Ryosuke Niwa 2018-03-08 23:14:21 PST
Comment on attachment 335060 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=335060&action=review

> Websites/perf.webkit.org/tools/js/buildbot-syncer.js:24
> +        this._buildRequestId = rawData['properties'] && rawData['properties'][syncer._buildRequestPropertyName] ?
> +                                   rawData['properties'][syncer._buildRequestPropertyName][0] : null;

Nit: Wrong indentation. '?' should be on line 24, and '?' should appear exactly 4 spaces to the right of this._buildRequestId.
Comment 6 Aakash Jain 2018-03-09 14:50:37 PST
Created attachment 335468 [details]
Patch for landing

Also passed workerName parameter to samplePendingBuildRequestData, instead of hard-coding worker name inside this sample data. This matches with samplePendingBuildRequestDeprecated.
Comment 7 Aakash Jain 2018-03-09 15:48:57 PST
Committed r229489: <https://trac.webkit.org/changeset/229489>
Comment 8 Radar WebKit Bug Importer 2018-03-12 17:17:22 PDT
<rdar://problem/38397677>
Comment 9 Radar WebKit Bug Importer 2018-03-12 17:17:23 PDT
<rdar://problem/38397676>