Bug 174196 – Write a support script to enable buildbot to upload to S3

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Lucas Forschler]

We'd like to use Amazon S3 storage for build artifacts. Teach it.

[Lucas Forschler]

<rdar://problem/32653786>

[Radar WebKit Bug Importer]

<rdar://problem/33150682>

[Lucas Forschler]

Created attachment 314696 [details]
v1 patch for review

[Lucas Forschler]

Created attachment 314707 [details]
v2 patch for review

[Lucas Forschler]

Created attachment 314761 [details]
v3 patch for review

[Stephanie Lewis]

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?

[Lucas Forschler]

Created attachment 314799 [details]
v4, fixed argparser from feedback

[Stephanie Lewis]

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

[Lucas Forschler]

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

[Lucas Forschler]

Created attachment 314802 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 314802 [details]
patch for landing

Clearing flags on attachment: 314802

Committed r219246: <http://trac.webkit.org/changeset/219246>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.