Closed
Bug 1135700
Opened 9 years ago
Closed 9 years ago
Switch "make uploadsymbols" to use Socorro Symbol Upload API
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(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)
People
(Reporter: ted, Assigned: ted)
References
Details
Attachments
(2 files, 1 obsolete file)
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.
Assignee | ||
Comment 1•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f152208e995
Assignee | ||
Comment 2•9 years ago
|
||
/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)
Assignee | ||
Comment 3•9 years ago
|
||
I'm importing the requests module because it makes the actual HTTP upload here a 1-liner.
Comment 4•9 years ago
|
||
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!
Comment 5•9 years ago
|
||
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 6•9 years ago
|
||
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+
Comment 7•9 years ago
|
||
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.
Assignee | ||
Comment 8•9 years ago
|
||
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.
Comment 9•9 years ago
|
||
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.
Assignee | ||
Comment 10•9 years ago
|
||
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.
Assignee | ||
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6512200e17e2 https://hg.mozilla.org/integration/mozilla-inbound/rev/db9a70fc3230
https://hg.mozilla.org/mozilla-central/rev/6512200e17e2 https://hg.mozilla.org/mozilla-central/rev/db9a70fc3230
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment 13•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-esr38/rev/73e7ea0802d0 https://hg.mozilla.org/releases/mozilla-esr38/rev/7dc6a6e6b324
status-firefox-esr38:
--- → fixed
Flags: in-testsuite-
Comment 14•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/6ea28bd2c785 https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/90fe773c90cc
status-b2g-v2.2:
--- → fixed
Comment 15•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/d5d37ea1491e https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/e13e64329b54
status-b2g-v2.1:
--- → fixed
Comment 16•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/4d33841d1681 https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/f934e0601b3f
status-b2g-v2.0:
--- → fixed
Comment 17•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-esr31/rev/5a75a78796d3 https://hg.mozilla.org/releases/mozilla-esr31/rev/88b5ccc921ba
status-firefox-esr31:
--- → fixed
Comment 18•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/d5d37ea1491e https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/e13e64329b54
Comment 19•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/4d33841d1681 https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/f934e0601b3f
status-b2g-v2.0M:
--- → fixed
Updated•9 years ago
|
status-b2g-v2.1S:
--- → fixed
status-b2g-master:
--- → fixed
Assignee | ||
Comment 20•9 years ago
|
||
Attachment #8568101 -
Attachment is obsolete: true
Attachment #8619561 -
Flags: review+
Attachment #8619562 -
Flags: review+
Assignee | ||
Comment 21•9 years ago
|
||
Assignee | ||
Comment 22•9 years ago
|
||
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•