Closed
Bug 1424651
Opened 6 years ago
Closed 6 years ago
Re-allow specifying symbol upload symbol token via a file.
Categories
(Firefox Build System :: General, enhancement)
Firefox Build System
General
Tracking
(firefox59 fixed)
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: tomprince, Assigned: tomprince)
References
Details
Attachments
(5 files)
The upload symbol code switched to fetching secrets from taskcluster in Bug 1422740. Thunderbird is still (although hopefully not for much longer) using buildbot to build nightlies, so I would like to re-add support for getting the token from a local file as well.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 6•6 years ago
|
||
mozreview-review |
Comment on attachment 8936206 [details] Bug 1424651: Factor out getting taskcluster secret; .mielczarek https://reviewboard.mozilla.org/r/206972/#review212874
Attachment #8936206 -
Flags: review?(ted) → review+
Comment 7•6 years ago
|
||
mozreview-review |
Comment on attachment 8936207 [details] Bug 1424651: Re-allow specifying symbol upload symbol token via a file; .mielczarek https://reviewboard.mozilla.org/r/206974/#review212876 ::: toolkit/crashreporter/tools/upload_symbols.py:87 (Diff revision 1) > + else: > + log.error('You must set the SYMBOL_SECRET or SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE environment variables!') > + return 1 > > - if os.environ.get('MOZ_SCM_LEVEL', '1') == '1': > + # Allow overwriting of the upload url with an environmental variable > + if 'SOCORRO_SYMBOL_UPLOAD_URL' in os.environ: Do you actually need to override the symbol upload URL? I removed this because I didn't think it was actually useful.
Attachment #8936207 -
Flags: review?(ted) → review+
Comment 8•6 years ago
|
||
mozreview-review |
Comment on attachment 8936208 [details] Bug 1424651: Fix upload_symbols.py documentation; .mielczarek https://reviewboard.mozilla.org/r/206976/#review212878 ::: toolkit/crashreporter/tools/upload_symbols.py:11 (Diff revision 1) > # > # This script uploads a symbol zip file passed on the commandline > # to the Tecken symbol upload API at https://symbols.mozilla.org/ . > # > +# Using this script requires you to have generated an authentication > # token in the Tecken web interface. You must store the token in a Taskcluster Might be good to rephrase this a little, like "You can either ..."
Attachment #8936208 -
Flags: review?(ted) → review+
Comment 9•6 years ago
|
||
mozreview-review |
Comment on attachment 8936209 [details] Bug 1424651: Remove unused SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE mozconfig variable; .mielczarek https://reviewboard.mozilla.org/r/206978/#review212892 Hah, I didn't even notice those variables were still there! ::: Makefile.in:306 (Diff revision 1) > buildsymbols: > endif > > uploadsymbols: > ifdef MOZ_CRASHREPORTER > -ifdef SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE > +ifneq ($(strip $(SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE)$(SYMBOL_SECRET),) I'd just remove this conditional entirely. The script should produce a useful error message if someone screws it up.
Attachment #8936209 -
Flags: review?(ted) → review+
Assignee | ||
Comment 10•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8936207 [details] Bug 1424651: Re-allow specifying symbol upload symbol token via a file; .mielczarek https://reviewboard.mozilla.org/r/206974/#review212876 > Do you actually need to override the symbol upload URL? I removed this because I didn't think it was actually useful. Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1424236#c5 the token available on buildbot won't work with the new URL. I'd like to avoid needing to change that token, particularly as I'd need to figure out if that will affect esr52 builds.
Assignee | ||
Comment 11•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8936208 [details] Bug 1424651: Fix upload_symbols.py documentation; .mielczarek https://reviewboard.mozilla.org/r/206976/#review212878 > Might be good to rephrase this a little, like "You can either ..." I'm happy to change it. My original thought was to put the prefered and then mention the deprecated one after.
Comment hidden (mozreview-request) |
Comment 13•6 years ago
|
||
Pushed by mozilla@hocat.ca: https://hg.mozilla.org/integration/autoland/rev/189fd4f1df41 Factor out getting taskcluster secret; r=ted.mielczarek https://hg.mozilla.org/integration/autoland/rev/6038fb7b458c Re-allow specifying symbol upload symbol token via a file; r=ted.mielczarek https://hg.mozilla.org/integration/autoland/rev/746d96792d18 Fix upload_symbols.py documentation; r=ted.mielczarek https://hg.mozilla.org/integration/autoland/rev/10ebf78f32bb Remove unused SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE mozconfig variable; r=ted.mielczarek
Comment hidden (mozreview-request) |
Assignee | ||
Comment 15•6 years ago
|
||
Comment on attachment 8936212 [details] Bug 1424651: Move SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE to the Thunderbird specific mozconfig files; Can you land this after the m-c parts merge from autoland? Thanks.
Flags: needinfo?(jorgk)
Comment 16•6 years ago
|
||
mozreview-review |
Comment on attachment 8936207 [details] Bug 1424651: Re-allow specifying symbol upload symbol token via a file; .mielczarek https://reviewboard.mozilla.org/r/206974/#review212918 ::: toolkit/crashreporter/tools/upload_symbols.py:77 (Diff revision 1) > - log.error('You must set the SYMBOL_SECRET environment variable!') > - return 1 > - auth_token = get_taskcluster_secret(secret_name) > + auth_token = get_taskcluster_secret(secret_name) > + elif 'SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE' not in os.environ: > + token_file = os.environ['SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE'] > + just a quick request for clarification. should this part not say elif 'SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE' in os.environ: if it isn't in the os.environ, I don't see how you can get that value from the os.environ? furthermore, by moving to symbols... you just need to log on to the Tecken ui and get a new symbol done. Then afaik, since |make upload-symbols| goes to symbols.mozilla.org, there really isn't anything to do with buildbot. Just need to ensure that the new symbols file are on the workers.
Comment 17•6 years ago
|
||
Backed out 4 changesets (bug 1424651) as requested by tomprice r=backout on a CLOSED TREE https://hg.mozilla.org/integration/autoland/rev/4a6655fb34175c4bbbe95a4362e111d234813767 https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=10ebf78f32bbbe5a5f2c6bf3be8fd6cb70c612cd
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 22•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8936207 [details] Bug 1424651: Re-allow specifying symbol upload symbol token via a file; .mielczarek https://reviewboard.mozilla.org/r/206974/#review212918 > just a quick request for clarification. > should this part not say > elif 'SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE' in os.environ: > > if it isn't in the os.environ, I don't see how > you can get that value from the os.environ? > > furthermore, by moving to symbols... you just > need to log on to the Tecken ui and get a new > symbol done. > > Then afaik, since |make upload-symbols| goes to symbols.mozilla.org, there really isn't anything > to do with buildbot. Just need to ensure that > the new symbols file are on the workers. Yep. Thanks for catching this before it hit m-c. It requires some work to get a new token onto the buildbot workers, and verify that doing that doesn't impact old branches that haven't switch to symbols.mozilla.org. Since TB will only be on buildbot through 59 (hopefully), it is easier to add this option in temporarily than deal with adding a new puppet secret.
Comment 23•6 years ago
|
||
Pushed by mozilla@hocat.ca: https://hg.mozilla.org/integration/autoland/rev/0413e7241a3d Factor out getting taskcluster secret; r=ted.mielczarek https://hg.mozilla.org/integration/autoland/rev/5eaf58103579 Re-allow specifying symbol upload symbol token via a file; r=ted.mielczarek https://hg.mozilla.org/integration/autoland/rev/f64f4a99ffca Fix upload_symbols.py documentation; r=ted.mielczarek https://hg.mozilla.org/integration/autoland/rev/8d5b3c6a6387 Remove unused SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE mozconfig variable; r=ted.mielczarek
Comment 24•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0413e7241a3d https://hg.mozilla.org/mozilla-central/rev/5eaf58103579 https://hg.mozilla.org/mozilla-central/rev/f64f4a99ffca https://hg.mozilla.org/mozilla-central/rev/8d5b3c6a6387
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment 25•6 years ago
|
||
Pushed by mozilla@hocat.ca: https://hg.mozilla.org/comm-central/rev/02fd46566f88 Move SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE to the Thunderbird specific mozconfig files; r=me
Updated•6 years ago
|
Flags: needinfo?(jorgk)
Comment 27•6 years ago
|
||
Pushed by mozilla@hocat.ca: https://hg.mozilla.org/comm-central/rev/f14a2331480c Specify SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE in linux32 mozconfigs too; r=me
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
•