Bug 222050 – [git-webkit] Failure to retrieve commit in EWS

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Jonathan Bedard]

Occasionally, EWS is failing to retrieve commit information. We need to determine why. An example of this failure is: https://ews-build.webkit.org/#/builders/36/builds/28373.

[Radar WebKit Bug Importer]

<rdar://problem/74439957>

[Jonathan Bedard]

Created attachment 420668 [details]
Patch

[Aakash Jain]

Comment on attachment 420668 [details]
Patch

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

> Tools/Scripts/libraries/webkitscmpy/webkitscmpy/remote/git_hub.py:153
> +            sys.stderr.write("Request to '{}' returned status code '{}'".format(url, response.status_code))

Do we need to add '\n'?

[Jonathan Bedard]

Committed r273015 (234216@main): <https://commits.webkit.org/234216@main>

[Jonathan Bedard]

Reopening to attach new patch.

[Jonathan Bedard]

Created attachment 420759 [details]
Patch

[Aakash Jain]

Comment on attachment 420759 [details]
Patch

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

r+ with unit-tests fixed.

> Tools/CISupport/ews-build/loadConfig.py:54
> +        value = passwords.get(variable.lower().replace('_', '-'))

why replace?

> Tools/CISupport/ews-build/steps.py:55
> +GITHUB_COM_USERNAME = 'GITHUB_COM_USERNAME'

can rename this to GITHUB_USERNAME to be more readable.

[Jonathan Bedard]

Created attachment 420840 [details]
Patch

[Aakash Jain]

Comment on attachment 420840 [details]
Patch

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

> Tools/CISupport/ews-build/loadConfig.py:54
> +        value = passwords.get(variable.lower().replace('_', '-'))

why replace? why not just have GITHUB_USERNAME listed in passwords.json?

[Jonathan Bedard]

(In reply to Aakash Jain from comment #9)
> Comment on attachment 420840 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=420840&action=review
> 
> > Tools/CISupport/ews-build/loadConfig.py:54
> > +        value = passwords.get(variable.lower().replace('_', '-'))
> 
> why replace? why not just have GITHUB_USERNAME listed in passwords.json?

Was trying to match what we did for the results database's API key, I'm fine with the raw variable in passwords.json.

[Jonathan Bedard]

Created attachment 420854 [details]
Patch

[Aakash Jain]

Comment on attachment 420854 [details]
Patch

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

> Tools/CISupport/ews-build/loadConfig.py:53
> +    for variable in ['GITHUB_COM_USERNAME', 'GITHUB_COM_ACCESS_TOKEN']:

I prefer GITHUB_USERNAME everywhere for better readability, but that’s not a big deal.

[Aakash Jain]

Maybe we should also test this on uat instance.

[Jonathan Bedard]

(In reply to Aakash Jain from comment #12)
> Comment on attachment 420854 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=420854&action=review
> 
> > Tools/CISupport/ews-build/loadConfig.py:53
> > +    for variable in ['GITHUB_COM_USERNAME', 'GITHUB_COM_ACCESS_TOKEN']:
> 
> I prefer GITHUB_USERNAME everywhere for better readability, but that’s not a
> big deal.

Changing the environment variable would result in a bit of a mess for things already using it (like commits.webkit.org). The environment variable is derived from the domain name of the GitHub instance.

[Aakash Jain]

(In reply to Jonathan Bedard from comment #14)
> Changing the environment variable would result in a bit of a mess for things already using it (like commits.webkit.org). The environment variable is derived from the domain name of the GitHub instance.
I didn't know that. Let's use GITHUB_COM_USERNAME and GITHUB_COM_ACCESS_TOKEN everywhere than, to maintain consistency.

[Jonathan Bedard]

Created attachment 421209 [details]
Patch

[Jonathan Bedard]

Committed r273267 (234438@main): <https://commits.webkit.org/234438@main>

[Aakash Jain]

(In reply to Jonathan Bedard from comment #17)
> Committed r273267 (234438@main): <https://commits.webkit.org/234438@main>
Restarted buildbot to pick up this change.