Bug 174198 - Enabling uploading archives to S3
Summary: Enabling uploading archives to S3
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Lucas Forschler
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-07-06 01:36 PDT by Lucas Forschler
Modified: 2019-05-15 07:21 PDT (History)
6 users (show)

See Also:

Attachments
v1 patch for review (3.27 KB, patch)
2017-07-06 01:41 PDT, Lucas Forschler 		lforschler: commit-queue-
Details | Formatted Diff | Diff
v2 patch for review (3.36 KB, patch)
2017-07-06 13:48 PDT, Lucas Forschler 		lforschler: commit-queue-
Details | Formatted Diff | Diff
v3 patch for review (3.26 KB, patch)
2017-07-06 13:52 PDT, Lucas Forschler 		lforschler: commit-queue-
Details | Formatted Diff | Diff
v4 patch with feedback applied (11.59 KB, patch)
2017-07-06 15:00 PDT, Lucas Forschler 		slewis: review+
lforschler: commit-queue+
Details | Formatted Diff | Diff
patch for landing (12.39 KB, patch)
2017-07-06 20:35 PDT, Lucas Forschler 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) 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 Lucas Forschler 2017-07-06 01:36:53 PDT 
Teach buildbot to use S3 for archive storage.
Comment 1 Radar WebKit Bug Importer 2017-07-06 01:38:02 PDT 
<rdar://problem/33152999>
Comment 2 Lucas Forschler 2017-07-06 01:41:56 PDT 
Created attachment 314710 [details]
v1 patch for review
Comment 3 Lucas Forschler 2017-07-06 13:48:18 PDT 
Created attachment 314753 [details]
v2 patch for review
Comment 4 Lucas Forschler 2017-07-06 13:52:19 PDT 
Created attachment 314754 [details]
v3 patch for review

had a search/replace error in the last one.
Comment 5 Lucas Forschler 2017-07-06 13:57:32 PDT 
Comment on attachment 314754 [details]
v3 patch for review

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

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:790
> +    haltOnFailure = True

This should be transferring and transferred
Comment 6 Kocsen Chung 2017-07-06 14:29:52 PDT 
Comment on attachment 314754 [details]
v3 patch for review

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

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:265
> +class ArchiveMinifiedBuiltProduct(ArchiveBuiltProduct):

Nit: ArchiveMinifiedBuildProduct
(Build instead of Built)

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:294
> +    

Nit: add extra whitespace for a total of two newlines between class bodies.

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:787
> +    name = "transfer-to-s3"

I think it would be nice to adhere to other steps when it comes to the order of these variables.

Usually (from immediate context above) I see name, description,  descriptionDone and command declared first and succeed in that order.
The rest can go below those.
Comment 7 Lucas Forschler 2017-07-06 14:53:56 PDT 
Comment on attachment 314754 [details]
v3 patch for review

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

>> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:265
>> +class ArchiveMinifiedBuiltProduct(ArchiveBuiltProduct):
> 
> Nit: ArchiveMinifiedBuildProduct
> (Build instead of Built)

I'm following the convention of the parent class, which is "ArchiveBuiltProduct".
I think we should stay consistent here.

>> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:294
>> +    
> 
> Nit: add extra whitespace for a total of two newlines between class bodies.

I went back and forth on this... this file is NOT consistent... about half of the classes have one space, there other two between them.
Let's do a follow-up patch which adds two spaces everywhere.

>> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:787
>> +    name = "transfer-to-s3"
> 
> I think it would be nice to adhere to other steps when it comes to the order of these variables.
> 
> Usually (from immediate context above) I see name, description,  descriptionDone and command declared first and succeed in that order.
> The rest can go below those.

sounds reasonable.
Comment 8 Lucas Forschler 2017-07-06 15:00:46 PDT 
Created attachment 314759 [details]
v4 patch with feedback applied
Comment 9 WebKit Commit Bot 2017-07-06 18:40:41 PDT 
Comment on attachment 314759 [details]
v4 patch with feedback applied

Rejecting attachment 314759 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 314759, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
.webkit.org/git/WebKit
   f7690be..bffe022  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 219228 = f7690bed686173d0f489bfe42db410ad8803373f
r219229 = bffe0225c245062649b98d0475077cff818f4b6b
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/4066537
Comment 10 Lucas Forschler 2017-07-06 20:35:20 PDT 
Created attachment 314804 [details]
patch for landing
Comment 11 WebKit Commit Bot 2017-07-06 21:13:39 PDT 
Comment on attachment 314804 [details]
patch for landing

Clearing flags on attachment: 314804

Committed r219235: <http://trac.webkit.org/changeset/219235>
Comment 12 WebKit Commit Bot 2017-07-06 21:13:43 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 13 Csaba Osztrogonác 2017-07-31 14:38:54 PDT 
(In reply to WebKit Commit Bot from comment #11)
> Comment on attachment 314804 [details]
> patch for landing
> 
> Clearing flags on attachment: 314804
> 
> Committed r219235: <http://trac.webkit.org/changeset/219235>

It broke the mastercfg_unittest.py build step list test, please update it.
Comment 14 Csaba Osztrogonác 2017-08-01 23:45:59 PDT 
unit test fix landed in https://trac.webkit.org/changeset/220125/webkit