Bug 203263 - [ews] Download the build archive from master when download from S3 fails
Summary: [ews] Download the build archive from master when download from S3 fails
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:
 
Reported: 2019-10-22 12:47 PDT by Aakash Jain
Modified: 2021-01-25 10:18 PST (History)
4 users (show)

See Also:

Attachments
Patch (6.65 KB, patch)
2019-10-22 12:55 PDT, Aakash Jain 		no flags Details | Formatted Diff | Diff
Patch (6.60 KB, patch)
2019-10-22 13:23 PDT, Aakash Jain 		jbedard: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2019-10-22 12:47:23 PDT 
[ews] Download the build archive from master when download from S3 fails. This would act as a fallback option when S3 is having problems (like it is having currently).
Comment 1 Aakash Jain 2019-10-22 12:55:30 PDT 
Created attachment 381585 [details]
Patch
Comment 2 Jonathan Bedard 2019-10-22 13:04:53 PDT 
Comment on attachment 381585 [details]
Patch

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

> Tools/BuildSlaveSupport/ews-build/steps.py:1313
> +        if rc == FAILURE:

Should this be rc != SUCCESS?

> Tools/BuildSlaveSupport/ews-build/steps.py:1321
> +        WithProperties(EWS_URL + 'ews-archives.webkit.org/%(fullPlatform)s-%(architecture)s-%(configuration)s/%(patch_id)s.zip')]

This looks wrong. It would resolve to 'https://ews-build.webkit.org/ews-archives.webkit.org/....'
Comment 3 Aakash Jain 2019-10-22 13:23:33 PDT 
Comment on attachment 381585 [details]
Patch

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

>> Tools/BuildSlaveSupport/ews-build/steps.py:1313
>> +        if rc == FAILURE:
> 
> Should this be rc != SUCCESS?

This is fine, we don't want to add that step in case of WARNINGS, SKIPPED or EXCEPTION.

We use similar logic at other places as well, e.g.: https://trac.webkit.org/browser/webkit/trunk/Tools/BuildSlaveSupport/ews-build/steps.py#L108

>> Tools/BuildSlaveSupport/ews-build/steps.py:1321
>> +        WithProperties(EWS_URL + 'ews-archives.webkit.org/%(fullPlatform)s-%(architecture)s-%(configuration)s/%(patch_id)s.zip')]
> 
> This looks wrong. It would resolve to 'https://ews-build.webkit.org/ews-archives.webkit.org/....'

And that's why we have code-reviews. Thanks for noticing. Fixed in updated patch.
Comment 4 Aakash Jain 2019-10-22 13:23:44 PDT 
Created attachment 381587 [details]
Patch
Comment 5 Aakash Jain 2019-10-22 13:27:03 PDT 
Sample run: https://ews-build.webkit-uat.org/#/builders/4/builds/12
Comment 6 Aakash Jain 2019-10-22 13:40:09 PDT 
Committed r251450: <https://trac.webkit.org/changeset/251450>
Comment 7 Radar WebKit Bug Importer 2019-10-22 13:41:18 PDT 
<rdar://problem/56513673>
Comment 8 Aakash Jain 2019-10-22 17:08:16 PDT 
Deployed in production few hours back.

Example of situation where this change helped: https://ews-build.webkit.org/#/builders/14/builds/5251