Bug 183194

Summary: BuildbotBuildEntry for buildbot 0.9 uses incorrect buildrequestid
Product: WebKit Reporter: Aakash Jain <aakash_jain>
Component: Tools / TestsAssignee: Aakash Jain <aakash_jain>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, commit-queue, dean_johnson, dewei_zhu, lforschler, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 175969    
Attachments:
Description Flags
Patch - WIP
none
Proposed patch
rniwa: review+
Patch for landing none

Aakash Jain
Reported 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.
Attachments
Patch - WIP (5.17 KB, patch)
2018-03-04 11:30 PST, Aakash Jain 		no flags
DetailsFormatted DiffDiff
Proposed patch (5.37 KB, patch)
2018-03-05 17:51 PST, Aakash Jain 		rniwa: review+
DetailsFormatted DiffDiff
Patch for landing (5.62 KB, patch)
2018-03-09 14:50 PST, Aakash Jain 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Aakash Jain
Comment 1 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.
Aakash Jain
Comment 2 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
Aakash Jain
Comment 3 2018-03-04 11:30:57 PST
Created attachment 334982 [details] Patch - WIP
Aakash Jain
Comment 4 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.
Ryosuke Niwa
Comment 5 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.
Aakash Jain
Comment 6 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.
Aakash Jain
Comment 7 2018-03-09 15:48:57 PST
Committed r229489: <https://trac.webkit.org/changeset/229489>
Radar WebKit Bug Importer
Comment 8 2018-03-12 17:17:22 PDT
<rdar://problem/38397677>
Radar WebKit Bug Importer
Comment 9 2018-03-12 17:17:23 PDT
<rdar://problem/38397676>
Note You need to log in before you can comment on or make changes to this bug.