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

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>