[mozlog] Add support for Python 3
Categories
(Testing :: Mozbase, enhancement, P3)
Tracking
(firefox67 fixed)
Tracking | Status | |
---|---|---|
firefox67 | --- | fixed |
People
(Reporter: davehunt, Assigned: slepushkin, Mentored)
References
Details
(Keywords: good-first-bug)
Attachments
(1 file, 1 obsolete file)
(deleted),
text/x-phabricator-request
|
Details |
Updated•6 years ago
|
Reporter | ||
Comment 1•6 years ago
|
||
Comment 2•6 years ago
|
||
Reporter | ||
Comment 3•6 years ago
|
||
Comment 4•6 years ago
|
||
Reporter | ||
Comment 5•6 years ago
|
||
Comment 6•6 years ago
|
||
Reporter | ||
Comment 7•6 years ago
|
||
Comment 8•6 years ago
|
||
Reporter | ||
Comment 9•6 years ago
|
||
Comment 10•6 years ago
|
||
Reporter | ||
Comment 11•6 years ago
|
||
Comment 12•6 years ago
|
||
Reporter | ||
Comment 13•6 years ago
|
||
Comment 14•6 years ago
|
||
Reporter | ||
Comment 15•6 years ago
|
||
Reporter | ||
Comment 16•6 years ago
|
||
Assignee | ||
Comment 17•6 years ago
|
||
Reporter | ||
Comment 18•6 years ago
|
||
Assignee | ||
Comment 19•6 years ago
|
||
Assignee | ||
Comment 20•6 years ago
|
||
Comment 21•6 years ago
|
||
Reporter | ||
Comment 22•6 years ago
|
||
Reporter | ||
Updated•6 years ago
|
Comment 23•6 years ago
|
||
Updated•6 years ago
|
Assignee | ||
Comment 24•6 years ago
|
||
Reporter | ||
Comment 25•6 years ago
|
||
Comment 26•6 years ago
|
||
Sure thing! I will submit a new try run for the updated patch.
:davehunt can you please remind me which test suites would be important to include in the try run?
Reporter | ||
Comment 27•6 years ago
|
||
(In reply to Raphael Pierzina [:raphael] UTC+01:00 from comment #26)
Sure thing! I will submit a new try run for the updated patch.
:davehunt can you please remind me which test suites would be important to include in the try run?
You can see the tasks that ran in your previous try run by looking at the try_task_config.json in the commit: https://hg.mozilla.org/try/rev/d05c5bf8f50176c8c351a607154e42ba53e4c501
Because lots of things potentially use mozlog I would all of the python suites, plus builds on windows and linux, 64bit opt and debug maybe. It looks like you included web platform tests last time, but I'm not convinced they use mozlog from the source.
Comment 28•6 years ago
|
||
(In reply to Dave Hunt [:davehunt] [he/him] ⌚️UTC from comment #27)
maybe. It looks like you included web platform tests last time, but I'm not convinced they use mozlog from the source.
They do. Especially the structured logger.
Comment 29•6 years ago
|
||
Here are the test results for the try run of the updated patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=357980fc3283435ccafad79856f4287c7dd9a054
Comment 30•6 years ago
|
||
(In reply to Raphael Pierzina [:raphael] UTC+01:00 from comment #29)
Here are the test results for the try run of the updated patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=357980fc3283435ccafad79856f4287c7dd9a054
There appears to be a minor linter error with the flake8 job but aside from that things look pretty good:
Assignee | ||
Comment 31•6 years ago
|
||
Hi, I've fixed linter error and submitted new revision
Comment 32•6 years ago
|
||
Thank you, Pavel! I will check out your updated patch tomorrow.
Comment 33•6 years ago
|
||
Try run for the updated patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7263bef1829062f15fc248fcb0f47d6a1432b15
Comment 34•6 years ago
|
||
Great work, Pavel! My attempt to land your patch was unsuccessful, because the commits could not be rebased automatically. If you could update your patch, that'd be fantastic. Otherwise I'll give it a try early next week. Thanks again for your contribution!
Reporter | ||
Comment 35•6 years ago
|
||
Updated•6 years ago
|
Reporter | ||
Comment 36•6 years ago
|
||
The only conflict was the version number. I resolved this and attempted to update the existing differential. Unfortunately this didn't work entirely as planned and a new differential was created. This was most likely due to a change I made to the commit message to reflect Raphael as the reviewer. I've abandoned the previous differential, and preserved Pavel as the author on the new one.
Comment 37•6 years ago
|
||
Comment 38•6 years ago
|
||
Backed out for awsy failures.
Backout link: https://hg.mozilla.org/integration/autoland/rev/c936686484a1a001f00e2f6a84a72fb0e19fc61e
Push link: https://hg.mozilla.org/integration/autoland/rev/c6f5a583ce62c9b38f6a928f516fb5c92d804a1c
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=225219881&repo=autoland&lineNumber=585
Comment 39•6 years ago
|
||
Comment 40•6 years ago
|
||
bugherder |
Comment 41•6 years ago
|
||
I just published mozlog 4.0 on PyPI: https://pypi.org/project/mozlog/4.0/
Comment 42•6 years ago
|
||
Thanks for doing this!
I wonder why we bumped the major version here. Per previous discussion we are attempting to use semver for mozbase, so I just spent some time trying to figure out which API changes that would affect consumers. As far as I can tell there aren't any, so I think this should have been a normal point release. I'm I'm wrong and there are some (maybe we now only accept one of bytes/unicode in some APIs?) that should definitely be documented at the very least in the commit message.
Reporter | ||
Comment 43•6 years ago
|
||
(In reply to James Graham [:jgraham] from comment #42)
I wonder why we bumped the major version here. Per previous discussion we are attempting to use semver for mozbase, so I just spent some time trying to figure out which API changes that would affect consumers. As far as I can tell there aren't any, so I think this should have been a normal point release. I'm I'm wrong and there are some (maybe we now only accept one of bytes/unicode in some APIs?) that should definitely be documented at the very least in the commit message.
I've been suggesting major version bumps for Python 3 support as it feels significant enough to merit it. I agree this doesn't adhere to strict semver, and perhaps I'm being over cautious with this suggestion. Sorry for causing confusion and wasting your time.
Reporter | ||
Updated•6 years ago
|
Assignee | ||
Comment 44•6 years ago
|
||
Hello.
Sorry, I was on a leave and afk. Is something needed from my side?
Reporter | ||
Comment 45•6 years ago
|
||
(In reply to Pavel Slepushkin from comment #44)
Sorry, I was on a leave and afk. Is something needed from my side?
Nothing left to do - we had to rebase your patch and resolve a conflict, but we have preserved your author credit. Thanks for the patch! :D
Description
•