[ews-build] Download archives from S3

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Aakash Jain]

https://bugs.webkit.org/show_bug.cgi?id=197922 added support to upload archives to S3. We should start downloading these archives from S3 (instead of downloading them from build-master).

[Aakash Jain]

Created attachment 370044 [details]
Patch

[EWS Watchlist]

Attachment 370044 [details] did not pass style-queue:


ERROR: Tools/BuildSlaveSupport/ews-build/steps.py:796:  [DownloadBuiltProduct.getResultSummary] Use of super on an old style class  [pylint/E1002] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps.py:797:  [DownloadBuiltProduct.getResultSummary] Instance of 'DownloadBuiltProduct' has no 'results' member  [pylint/E1101] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:1038:  [TestDownloadBuiltProduct.test_failure] Passing unexpected keyword argument 'state_string' in function call  [pylint/E1123] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:1038:  [TestDownloadBuiltProduct.test_failure] No value passed for parameter 'status_text' in function call  [pylint/E1120] [5]
Total errors found: 4 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jonathan Bedard]

Comment on attachment 370044 [details]
Patch

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

> Tools/BuildSlaveSupport/ews-build/steps.py:796
> +    def getResultSummary(self):

Why didn't we have something like this previously? It looks like most of this change is just swapping out the url.

> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:1017
> +                        command=['python', 'Tools/BuildSlaveSupport/download-built-product', '--platform=ios', '--release', 'https://s3-us-west-2.amazonaws.com/ews-archives.webkit.org/ios-simulator-12-x86_64-release/1234.zip'],

It feels like download-built-product is missing something. When I look at this script, it doesn't do anything with platform. It makes me think that the iOS simulator bit isn't actually going to work. I think we need to put the binaries into WebKitBuild/<configuration>-iphonesimulator.

[Aakash Jain]

> Why didn't we have something like this previously?
I have been polishing these strings (e.g.: https://bugs.webkit.org/show_bug.cgi?id=195963, https://bugs.webkit.org/show_bug.cgi?id=195945, https://bugs.webkit.org/show_bug.cgi?id=195995), but this has been a low priority, especially the strings (like this one) which doesn't appear frequently.

> It looks like most of this change is just swapping out the url.
Right.
> > Tools/BuildSlaveSupport/ews-build/steps_unittest.py:1017
> > +                        command=['python', 'Tools/BuildSlaveSupport/download-built-product', '--platform=ios', '--release', 'https://s3-us-west-2.amazonaws.com/ews-archives.webkit.org/ios-simulator-12-x86_64-release/1234.zip'],
> 
> It feels like download-built-product is missing something. When I look at this script, it doesn't do anything with platform.
Yeah, it only check if platform is windows or not. Do you prefer to remove this parameter? I kept it as it would be consistent with similar build.webkit.org step.

>  It makes me think that the iOS simulator bit isn't actually going to work. I think we need to put the binaries into WebKitBuild/<configuration>-iphonesimulator.
It actually works, tested in https://ews-build.webkit-uat.org/#/builders/20/builds/2636 . The uploaded archive itself has the subfolder "<configuration>-iphonesimulator.' You can verify by downloading and unzipping this: https://s3-us-west-2.amazonaws.com/ews-archives.webkit.org/ios-simulator-12-x86_64-release/369972.zip

[Jonathan Bedard]

(In reply to Aakash Jain from comment #4)
> ...
> > It feels like download-built-product is missing something. When I look at this script, it doesn't do anything with platform.
> Yeah, it only check if platform is windows or not. Do you prefer to remove
> this parameter? I kept it as it would be consistent with similar
> build.webkit.org step.

I actually would prefer this variable be removed if it's not used.

> ... 
> >  It makes me think that the iOS simulator bit isn't actually going to work. I think we need to put the binaries into WebKitBuild/<configuration>-iphonesimulator.
> It actually works, tested in
> https://ews-build.webkit-uat.org/#/builders/20/builds/2636 . The uploaded
> archive itself has the subfolder "<configuration>-iphonesimulator.' You can
> verify by downloading and unzipping this:
> https://s3-us-west-2.amazonaws.com/ews-archives.webkit.org/ios-simulator-12-
> x86_64-release/369972.zip

This part is surprising to me. It may just be surprising because this isn't how our internal archiving system works, but that may also be indicative of an architectural flaw in the WebKit archiving system. But since it's working, that's not in the scope of this patch.

[Aakash Jain]

Created attachment 370099 [details]
Patch

[EWS Watchlist]

Attachment 370099 [details] did not pass style-queue:


ERROR: Tools/BuildSlaveSupport/ews-build/steps.py:796:  [DownloadBuiltProduct.getResultSummary] Use of super on an old style class  [pylint/E1002] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps.py:797:  [DownloadBuiltProduct.getResultSummary] Instance of 'DownloadBuiltProduct' has no 'results' member  [pylint/E1101] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:1036:  [TestDownloadBuiltProduct.test_failure] Passing unexpected keyword argument 'state_string' in function call  [pylint/E1123] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:1036:  [TestDownloadBuiltProduct.test_failure] No value passed for parameter 'status_text' in function call  [pylint/E1120] [5]
Total errors found: 4 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Aakash Jain]

> I actually would prefer this variable be removed if it's not used.
Done

[WebKit Commit Bot]

Comment on attachment 370099 [details]
Patch

Clearing flags on attachment: 370099

Committed r245433: <https://trac.webkit.org/changeset/245433>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/50880970>