Bug 174873 – Dashboard: Add displayName property in BuildbotQueue

• Status: RESOLVED
• Resolution: WORKSFORME
• Component: Tools / Tests

[Aakash Jain]

BuildbotQueue has an 'id', which serves two purposes:
1) used as an id in URLs.
2) used as a display name in dashboard.

We should separate out these two use-cases. 

buildbot 0.9 have separate values for 'name' and 'id'. 'id' are integers in buildbot 0.9. This change will ease the buildbot 0.9 support.

[Aakash Jain]

Created attachment 316495 [details]
Proposed patch

[Alexey Proskuryakov]

Comment on attachment 316495 [details]
Proposed patch

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

> Tools/ChangeLog:3
> +        Dashboard: Add displayName property in BuildbotQueue

How is this different from the existing "heading" property?

[Aakash Jain]

> How is this different from the existing "heading" property?

Heading seems to be the high level category of the builder, example of few headings: "Production", "Debug", "Performance".

displayName is the complete name of the builder, e.g: "Apple El Capitan Release WK1 (Tests)". 

Currently we are using 'id' in the places where this patch use displayName. This was fine in buildbot 0.8 since displayName can be used as an id. However, in buildbot 0.9 name and id are separate.

[Alexey Proskuryakov]

I don't think that's all there is to it. See e.g. this part of WebKitBuildbot.js:

        "Apple El Capitan JSC": {platform: Dashboard.Platform.MacOSXElCapitan, heading: "JavaScript", combinedQueues: {
            "Apple El Capitan 32-bit JSC (BuildAndTest)": {heading: "32-bit JSC (BuildAndTest)"},
...

Perhaps heading is only implemented for combined queues at this time?

[Aakash Jain]

> Perhaps heading is only implemented for combined queues at this time?

Yeah, heading seems to be defined for some of the queues (24 out of ~50 queues in WebKitBuild.js), and for many of queues it is just one-word.

The purpose of this patch is to ensure that the variable 'id' is used only as an 'id' (e.g.: in URLs for REST API). 'id' should not be used as a display name. This would simplify the support for buildbot 0.9, in which id is an integer.

This point came up in discussion with Dan Bates, in order to avoid adding a new variable 'builderid' in BuildbotQueue in https://bugs.webkit.org/attachment.cgi?id=316276&action=review

[Alexey Proskuryakov]

Anyway, my comment is that given that heading is currently used in the same way as opposed displayName, I think that we shouldn't be using two properties with different names. I haven't looked into any other aspects of the patch.

[Aakash Jain]

In further discussions with Dan Bates, we figured out a better way to handle buildbot 0.9 support which doesn't require this change.