Closed
Bug 1501802
Opened 6 years ago
Closed 6 years ago
Update tooltool.py fetch_file to log.info exceptions instead of log.debug
Categories
(Release Engineering :: General, enhancement)
Release Engineering
General
Tracking
(firefox-esr60 fixed, firefox65 fixed)
RESOLVED
FIXED
People
(Reporter: bc, Assigned: bc)
References
Details
Attachments
(1 file, 2 obsolete files)
2.40 KB,
patch
|
garbas
:
review+
|
Details | Diff | Splinter Review |
In bug 1499246 we have been chasing down issues with downloading binaries from tooltool at Bitbar. I finally got a pointer to where the problem might be occurring once I made a change to log exceptions in fetch_file. I think this would be a good change to make in general to help in diagnosing issues. I'll attach my wip/demo patch for your perusal.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → bob
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•6 years ago
|
||
I found this very useful. Let's make it official.
Attachment #9019814 -
Attachment is obsolete: true
Attachment #9020349 -
Flags: review?(rgarbas)
Comment 2•6 years ago
|
||
Comment on attachment 9020349 [details] [diff] [review]
tooltool-fetch-logging.patch
Review of attachment 9020349 [details] [diff] [review]:
-----------------------------------------------------------------
would using log.info(..., exc_info=True) be a better way of logging the exceptions?
Assignee | ||
Comment 3•6 years ago
|
||
I can try it out and see how it does. Give me a bit and I'll report back.
Assignee | ||
Comment 4•6 years ago
|
||
I wonder though if that will introduce oranges due to the exception stack in the logs?
Assignee | ||
Comment 5•6 years ago
|
||
I ran a full test run on try <https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&tier=1%2C2%2C3&revision=f13f8079d4c4af6e0a352be2dde454cf09d78dab> with an additional patch <https://hg.mozilla.org/try/rev/80ebf7f1cdf55a9b867e6e7166a0078c58bb8afe>
I didn't see this failure once so it is not so common as to turn a things perma orange right away. I'm not sure we want to change the behavior to turn these into oranges though, but I could see how that could be a good thing. I'll let you make that call. Let me know and if you like, I'll fold it into the original patch and submit that for review.
Assignee | ||
Comment 6•6 years ago
|
||
I did see this while testing the android hardware which is where I originally found this issue:
<https://treeherder.mozilla.org/logviewer.html#?job_id=208148925&repo=try&lineNumber=1836-1853>
Assignee | ||
Comment 7•6 years ago
|
||
folded exc_info=True into the patch.
Attachment #9020349 -
Attachment is obsolete: true
Attachment #9020349 -
Flags: review?(rgarbas)
Attachment #9022033 -
Flags: review?(rgarbas)
Comment 8•6 years ago
|
||
Comment on attachment 9022033 [details] [diff] [review]
tooltool-fetch-logging.patch
:bc sorry for late review, it was a busy week
can you also apply the same patch also to https://github.com/mozilla/build-tooltool/blob/master/tooltool.py
Attachment #9022033 -
Flags: review?(rgarbas) → review+
Assignee | ||
Comment 9•6 years ago
|
||
https://github.com/mozilla/build-tooltool/pull/45 with a change to remove the extraneous e variable and after some git foobars.
Comment 10•6 years ago
|
||
Pushed by bclary@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4034fbcc0a55
Update tooltool.py fetch_file to log.info exceptions instead of log.debug, r=garbas
Assignee | ||
Comment 11•6 years ago
|
||
Note I removed the extraneous e from the version I pushed as well.
Comment 12•6 years ago
|
||
Backout by csabou@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a725d6063ad0
Backed out changeset 4034fbcc0a55 for causing mass mochitest failures.
Assignee | ||
Comment 13•6 years ago
|
||
This was due to the rebuilding of the docker images and bug 1503756 I believe. I should have caught it if I had done a full set of unit tests on try when the image was rebuilt.
Depends on: 1503756
Comment 14•6 years ago
|
||
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/565215cf2e5e
Update tooltool.py fetch_file to log.info exceptions instead of log.debug, r=garbas
Comment 15•6 years ago
|
||
Backout by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/48720735b142
Backed out changeset 565215cf2e5e for causing mass mda failures. CLOSED TREE
Comment 16•6 years ago
|
||
Bug 1503756 was pushed here https://bugzilla.mozilla.org/show_bug.cgi?id=1503756#c31 and eventually got backed out. After that backout the changes pushed here went on creating new mass failures on mda test.
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=211901297&repo=autoland&lineNumber=5167
Backout push for 1503756: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=mda&group_state=expanded&selectedJob=211879519&revision=2a9517f92a0d29bd8280e7f6fd897f19c4cf68c3
Flags: needinfo?(bob)
Assignee | ||
Comment 17•6 years ago
|
||
I didn't land this. I think gps landed it as part of the attempt to fix the docker image building. If it is landed again as part of the docker image fix, and the docker image fix is backed out this should be backed out at the same time as it generates a new docker image which will also fail.
Flags: needinfo?(bob)
Comment 18•6 years ago
|
||
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9a627c744233
Update tooltool.py fetch_file to log.info exceptions instead of log.debug, r=garbas
Comment 19•6 years ago
|
||
bugherder |
Comment 20•6 years ago
|
||
bugherder uplift |
status-firefox-esr60:
--- → fixed
Updated•3 years ago
|
Component: Applications: ToolTool → General
You need to log in
before you can comment on or make changes to this bug.
Description
•