Switch "make uploadsymbols" to use Socorro Symbol Upload API

RESOLVED FIXED in Firefox 39

Status

defect
RESOLVED FIXED
4 years ago
Last year

People

(Reporter: ted, Assigned: ted)

Tracking

Trunk
mozilla39
Dependency tree / graph

Firefox Tracking Flags

(firefox39 fixed, firefox-esr31 fixed, firefox-esr38 fixed, b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.1S fixed, b2g-v2.2 fixed, b2g-master fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

We're going to switch everything that's uploading symbols to using the Socorro symbol upload API (see the bug this blocks and the dependency tree). Socorro has a simple REST API for uploading symbols, so this should not be a lot of code.

Testing this is kind of a pain because it's Nightly/Release only, and a) we don't have a way to test nightlies on try, and b) even if we did the try pool is separate from the build pool so they wouldn't have the right authentication token available. The current plan of action is to add this upload method alongside the existing SSH upload method, initially pointed at the Socorro staging server to shake out any issues. After that looks okay we'll point it at production but still run both upload methods for a short time (we're syncing the NFS mount to S3 so there will be duplication but it's not a problem on Socorro's end). Once we're comfortable that it's working we'll remove the old SSH upload method.
Posted file MozReview Request: bz://1135700/ted (obsolete) —
/r/4187 - bug 1135700 - Import the Python requests module
/r/4189 - bug 1135700 - make uploadsymbols use Socorro symbol upload API

Pull down these commits:

hg pull review -r 0f152208e9951157623ff57dbe00573ebef7ebff
Attachment #8568101 - Flags: review?(gps)
I'm importing the requests module because it makes the actual HTTP upload here a 1-liner.
https://reviewboard.mozilla.org/r/4187/#review3335

A decent HTTP API in the tree \o/. I can actually make use of this in bug 1132771, since I would like to write a mach command that audits Bugzilla compoonents and code reviews against what's actually configured on bugzilla.mozilla.org!
https://reviewboard.mozilla.org/r/4189/#review3337

While I raised a few issues, none are significant enough to block landing.

::: toolkit/crashreporter/tools/upload_symbols.py
(Diff revision 1)
> +#

Please add a quick comment to this file indicating its purpose.

::: toolkit/crashreporter/tools/upload_symbols.py
(Diff revision 1)
> +    token_file = substs['SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE']

I hate the buildconfig module with passion. If you could pass in the path to the toke file via CLI args, I'd like this patch even better.

::: toolkit/crashreporter/tools/upload_symbols.py
(Diff revision 1)
> +    r = requests.post(
> +        url,
> +        files={'symbols.zip': open(sys.argv[1], 'rb')},
> +        headers={'Auth-Token': auth_token},
> +        allow_redirects=False
> +    )

Should we add an explicit timeout?

Also, consider throwing this behind a try..except requests.exceptions.RequestException.

::: toolkit/crashreporter/tools/upload_symbols.py
(Diff revision 1)
> +        files={'symbols.zip': open(sys.argv[1], 'rb')},

Using files= will use multi-part mime with base64 encoding, no? This will result in significant encoding overhead for symbols.zip. This will consume a lot of excessive bandwidth.

I don't suppose there's a way to transfer the data in raw/binary form to avoid the overhead? (I understand this may be out of your control and a follow-up bug.)
Comment on attachment 8568101 [details]
MozReview Request: bz://1135700/ted

https://reviewboard.mozilla.org/r/4185/#review3339

Ship It!
Attachment #8568101 - Flags: review?(gps) → review+
https://reviewboard.mozilla.org/r/4185/#review3341

I forgot to mention that your need to import `requests` is debatable. But I like having access to powerful libraries, so I'll allow it.
https://reviewboard.mozilla.org/r/4189/#review3351

> I hate the buildconfig module with passion. If you could pass in the path to the toke file via CLI args, I'd like this patch even better.

Any particular reason? For a single-purpose script I feel like having the knowledge of what files it needs baked in makes more sense than having to pass them as parameters. (Also it lets me easily write useful error messages here vs. having to do it in make.)

> Using files= will use multi-part mime with base64 encoding, no? This will result in significant encoding overhead for symbols.zip. This will consume a lot of excessive bandwidth.
> 
> I don't suppose there's a way to transfer the data in raw/binary form to avoid the overhead? (I understand this may be out of your control and a follow-up bug.)

The API accepts the file as a binary blob, but I'd also like to be able to send metadata along with it (which it doesn't currently accept) which would necessitate using multipart/form-data anyway.
https://reviewboard.mozilla.org/r/4189/#review3457

> Any particular reason? For a single-purpose script I feel like having the knowledge of what files it needs baked in makes more sense than having to pass them as parameters. (Also it lets me easily write useful error messages here vs. having to do it in make.)

mozbuild.base.MozbuildObject.from_environment() is preferred over buildconfig. buildconfig is a holdover from before we had nice APIs. It does get the job done in an easy-to-use way, I concede.

Don't worry about fixing this if you don't want to.

> The API accepts the file as a binary blob, but I'd also like to be able to send metadata along with it (which it doesn't currently accept) which would necessitate using multipart/form-data anyway.

I just wanted to raise the concern. As long as it is known to you and you are fine with that, it's ultimately your choice to use the inefficient encoding. Feel free to drop the issue.
https://reviewboard.mozilla.org/r/4189/#review3577

> Should we add an explicit timeout?
> 
> Also, consider throwing this behind a try..except requests.exceptions.RequestException.

Both good suggestions. I'm putting a 60 second timeout in there for now, the requests documentation says the timeout isn't global for the request, but only "have waited this long without a response from the server". We'll have to see if that's a problem in production, since the server side is going to be synchronously uploading the contents of the zip to S3.
https://hg.mozilla.org/mozilla-central/rev/6512200e17e2
https://hg.mozilla.org/mozilla-central/rev/db9a70fc3230
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Attachment #8568101 - Attachment is obsolete: true
Attachment #8619561 - Flags: review+
Attachment #8619562 - Flags: review+

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.