Bug 174196 - Write a support script to enable buildbot to upload to S3
Summary: Write a support script to enable buildbot to upload 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-05 22:00 PDT by Lucas Forschler
Modified: 2017-07-06 22:57 PDT (History)
4 users (show)

See Also:

Attachments
v1 patch for review (2.47 KB, patch)
2017-07-05 22:10 PDT, Lucas Forschler 		lforschler: commit-queue-
Details | Formatted Diff | Diff
v2 patch for review (2.48 KB, patch)
2017-07-06 01:18 PDT, Lucas Forschler 		lforschler: commit-queue-
Details | Formatted Diff | Diff
v3 patch for review (1.90 KB, patch)
2017-07-06 15:07 PDT, Lucas Forschler 		lforschler: commit-queue+
Details | Formatted Diff | Diff
v4, fixed argparser from feedback (1.95 KB, patch)
2017-07-06 20:12 PDT, Lucas Forschler 		slewis: review+
lforschler: commit-queue-
Details | Formatted Diff | Diff
patch for landing (2.66 KB, patch)
2017-07-06 20:27 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-05 22:00:39 PDT 
We'd like to use Amazon S3 storage for build artifacts. Teach it.
Comment 1 Lucas Forschler 2017-07-05 22:00:56 PDT 
<rdar://problem/32653786>
Comment 2 Radar WebKit Bug Importer 2017-07-05 22:02:08 PDT 
<rdar://problem/33150682>
Comment 3 Lucas Forschler 2017-07-05 22:10:28 PDT 
Created attachment 314696 [details]
v1 patch for review
Comment 4 Lucas Forschler 2017-07-06 01:18:24 PDT 
Created attachment 314707 [details]
v2 patch for review
Comment 5 Lucas Forschler 2017-07-06 15:07:01 PDT 
Created attachment 314761 [details]
v3 patch for review
Comment 6 Stephanie Lewis 2017-07-06 18:22:27 PDT 
Comment on attachment 314761 [details]
v3 patch for review

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

> Tools/BuildSlaveSupport/build.webkit.org-config/transfer-archive-to-s3:34
> +args = parser.parse_args()

Maybe add help messages.  It's not clear without reading the code that the archive arguments is not the path to the minified archive.  It might be more natural to allow both.

> Tools/BuildSlaveSupport/build.webkit.org-config/transfer-archive-to-s3:41
> +minifiedArchive = head + '/minified-' + tail

Wouldnt this fail if --archive wasn't passed?  Why is --archive optional?
Comment 7 Lucas Forschler 2017-07-06 20:12:11 PDT 
Created attachment 314799 [details]
v4, fixed argparser from feedback
Comment 8 Stephanie Lewis 2017-07-06 20:15:50 PDT 
Comment on attachment 314799 [details]
v4, fixed argparser from feedback

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

> Tools/BuildSlaveSupport/build.webkit.org-config/transfer-archive-to-s3:16
> +    s3.upload_file(archive_path, bucket, key)

I don't like using globals in functions.  Is there a reason to declare s3 as a global?

> Tools/BuildSlaveSupport/build.webkit.org-config/transfer-archive-to-s3:34
> +head, tail = os.path.split(str(args.archive))

Should probably check if args.archive has a value before calling split on it
Comment 9 Lucas Forschler 2017-07-06 20:21:53 PDT 
Comment on attachment 314799 [details]
v4, fixed argparser from feedback

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

>> Tools/BuildSlaveSupport/build.webkit.org-config/transfer-archive-to-s3:16
>> +    s3.upload_file(archive_path, bucket, key)
> 
> I don't like using globals in functions.  Is there a reason to declare s3 as a global?

would you rather instantiate each time we run through the loop?

>> Tools/BuildSlaveSupport/build.webkit.org-config/transfer-archive-to-s3:34
>> +head, tail = os.path.split(str(args.archive))
> 
> Should probably check if args.archive has a value before calling split on it

the argparser will fail if there is no value
Comment 10 Lucas Forschler 2017-07-06 20:27:43 PDT 
Created attachment 314802 [details]
patch for landing
Comment 11 WebKit Commit Bot 2017-07-06 22:57:37 PDT 
Comment on attachment 314802 [details]
patch for landing

Clearing flags on attachment: 314802

Committed r219246: <http://trac.webkit.org/changeset/219246>
Comment 12 WebKit Commit Bot 2017-07-06 22:57:38 PDT 
All reviewed patches have been landed.  Closing bug.