Closed Bug 1501802 Opened Last year Closed 11 months ago

Update tooltool.py fetch_file to log.info exceptions instead of log.debug

Categories

(Release Engineering :: Applications: ToolTool, enhancement)

enhancement
Not set

Tracking

(firefox-esr60 fixed, firefox65 fixed)

RESOLVED FIXED
Tracking Status
firefox-esr60 --- fixed
firefox65 --- fixed

People

(Reporter: bc, Assigned: bc)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch tooltool-fetch-logging.patch (obsolete) — 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: nobody → bob
Status: NEW → ASSIGNED
Attached patch tooltool-fetch-logging.patch (obsolete) — Splinter Review
I found this very useful. Let's make it official.
Attachment #9019814 - Attachment is obsolete: true
Attachment #9020349 - Flags: review?(rgarbas)
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?
I can try it out and see how it does. Give me a bit and I'll report back.
I wonder though if that will introduce oranges due to the exception stack in the logs?
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.
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>
folded exc_info=True into the patch.
Attachment #9020349 - Attachment is obsolete: true
Attachment #9020349 - Flags: review?(rgarbas)
Attachment #9022033 - Flags: review?(rgarbas)
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+
https://github.com/mozilla/build-tooltool/pull/45 with a change to remove the extraneous e variable and after some git foobars.
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
Note I removed the extraneous e from the version I pushed as well.
Backout by csabou@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a725d6063ad0
Backed out changeset 4034fbcc0a55 for causing mass mochitest failures.
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
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
Backout by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/48720735b142
Backed out changeset 565215cf2e5e for causing mass mda failures. CLOSED TREE
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)
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
https://hg.mozilla.org/mozilla-central/rev/9a627c744233
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.