Closed
Bug 1389592
Opened 7 years ago
Closed 7 years ago
Update Treeherder requirements files to be compatible with Python 3
Categories
(Tree Management :: Treeherder, defect, P3)
Tree Management
Treeherder
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: emorley, Assigned: dyex719, Mentored)
References
Details
(Keywords: good-first-bug, Whiteboard: [lang=py])
Attachments
(1 file, 2 obsolete files)
One of the pre-requisites for bug 1330474 is to make sure that the requirements files used by Treeherder are compatible with Python 3:
https://github.com/mozilla/treeherder/blob/master/requirements/common.txt
https://github.com/mozilla/treeherder/blob/master/requirements/dev.txt
There will likely be two sets of changes required:
a) For things that are not needed on Python 3 (for example: enum34, functools32, futures [1]) - use the special syntax to denote which python version it applies to:
`package-name; python_version < '3'`
b) Adding additional hashes for some packages, to prevent the "hash doesn't match" error. (Some packages will have different archives for Python 2 vs Python 3, so the hash will vary). Copy the new hash from the error and add it to those listed in the requirements file.
To work on this you don't need a full Treeherder Vagrant environment, but can just:
1) Install Python 3.6 (Python 3.5 will probably be fine too)
2) Create a virtualenv
3) Clone the Treeherder repo (https://github.com/mozilla/treeherder)
4) From the root of the repo run `pip install -r requirements/common.txt -r requirements/dev.txt`
5) Open a PR with a title of form "Bug NNNNN - ..."
6) Mark me as a reviewer of the attachment that the bot automatically creates on this bug, after the PR was opened.
Reporter | ||
Comment 1•7 years ago
|
||
[1] Note the 'promise' package may be required until https://github.com/syrusakbary/promise/issues/40 is fixed.
Reporter | ||
Comment 2•7 years ago
|
||
Note: Your local pip version must be >8, otherwise it won't understand the hash syntax in our requirements files (https://pip.pypa.io/en/stable/reference/pip_install/#hash-checking-mode).
Reporter | ||
Comment 3•7 years ago
|
||
(In reply to Ed Morley [:emorley] from comment #0)
> There will likely be two sets of changes required:
Make that three.
c) Some packages will need additional dependencies, for example the `taskcluster` package will need `aiohttp` and some others.
Assignee | ||
Comment 4•7 years ago
|
||
Hello, this would be the second bug I'd be working on. Can you assign this to me?
Reporter | ||
Comment 5•7 years ago
|
||
Certainly!
Btw in step 4 above, I should have added "... and keep running this command and making changes until it completes with no errors".
If there's anything else that's not clear above just ask :-)
Assignee: nobody → dyex719
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•7 years ago
|
||
I'm having trouble locating packages that have been discontinued in python3.
When I open a pull request, what do I put as the base branch and the compare branch? Am I missing anything? Thank you!
Flags: needinfo?(emorley)
Attachment #8899090 -
Flags: review?(emorley)
Attachment #8899090 -
Flags: feedback+
Reporter | ||
Comment 7•7 years ago
|
||
Hi! I'm writing this on mobile so will just be a brief reply for now - I'll look at this in more detail when I'm back at work Monday.
The packages that are marked as version less than 3 don't actually need replacements - that functionality is now built into Python 3 itself.
For how to provide the changes, open a github pull request like so:
https://help.github.com/articles/about-pull-requests/
Make sure that both the commit message title and the pull request title contain this bug number, in the form given in step 5 above. By doing that, a bot will automatically add a link in this bug to it.
Flags: needinfo?(emorley)
Reporter | ||
Comment 8•7 years ago
|
||
Comment on attachment 8899090 [details] [diff] [review]
bug1389592.patch
Marking this as obsolete, since we use pull requests rather than patches. The bot will add a new attachment that acts as a link to the pull request, which can then be flagged for review :-)
Attachment #8899090 -
Attachment is obsolete: true
Attachment #8899090 -
Flags: review?(emorley)
Attachment #8899090 -
Flags: feedback+
Comment 9•7 years ago
|
||
Assignee | ||
Comment 10•7 years ago
|
||
(In reply to Autolander from comment #9)
> Created attachment 8899216 [details]
> [treeherder] Dyex719:patch-1 > mozilla:master
Please take a look at this.
Status: ASSIGNED → NEW
Flags: needinfo?(emorley)
Reporter | ||
Comment 11•7 years ago
|
||
I've left a comment on the PR :-)
Could you also update the commit message to something like:
"""
Bug 1389592 - Update requirements files to be compatible with Python 3
And add a Travis job to prevent regressions.
"""
...since it's currently:
"""
Bug 1389592
[Treeherder] Update the requirements file to be compatible with Python3. r=emorley
"""
(For the Treeherder project we tend to omit the "r=...", and whilst the bug summary contains the word "Treeherder", we typically exclude that from the commit message, since it's ending up in the treeherder repository, so everything there is known to be for Treeherder.)
When it's ready for re-review, flag me using the "review?" feature (under "details" on the attachment in this bug) and select me from the suggested reviewers - to make sure it ends up in my queue.
Many thanks!
Status: NEW → ASSIGNED
Flags: needinfo?(emorley)
Comment 12•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8902742 -
Flags: review?(emorley)
Reporter | ||
Comment 13•7 years ago
|
||
Comment on attachment 8902742 [details]
[treeherder] Dyex719:master > mozilla:master
I've left a comment on the new PR:
https://github.com/mozilla/treeherder/pull/2739#issuecomment-326268580
Marking this as obsolete since we can continue the review on the old PR (which already has an attachment linked above) :-)
Attachment #8902742 -
Attachment is obsolete: true
Attachment #8902742 -
Flags: review?(emorley)
Assignee | ||
Comment 14•7 years ago
|
||
The build passed all the tests.
I think I have created a new branch called Patch-1 other than the already existing branch Dyex719-Patch-1.
Sorry for the confusion.
Flags: needinfo?(emorley)
Reporter | ||
Comment 15•7 years ago
|
||
Comment on attachment 8899216 [details]
[treeherder] Dyex719:patch-1 > mozilla:master
This looks great! Many thanks for your help :-)
The next step in bug 1330474 is likely to look into different ways the code in the treeherder repository itself can be linted for Python 3 syntax compatibility (before we even start to try and run the tests against it) - should you be interested?
Flags: needinfo?(emorley)
Attachment #8899216 -
Flags: review+
Comment 16•7 years ago
|
||
Commit pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/f305816fd1759351ec9450e273c98f20883873c4
Bug 1389592 - Ensure requirements files are compatible with Python 3 (#2722)
Reporter | ||
Updated•7 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 17•7 years ago
|
||
Yep I'm interested! Thanks for your help! I'll look into the new steps soon :)
Assignee | ||
Comment 18•7 years ago
|
||
Can you guide me towards another bug? Should I work on https://bugzilla.mozilla.org/show_bug.cgi?id=1388018 or something else?
Reporter | ||
Comment 19•7 years ago
|
||
That bug would be good, or else once bug 1388013 is finished perhaps bug 1388018 and then bug 1388019? (Since Treeherder uses them, so will need them to be Python 3 compatible too). (You could probably make a start on them even before bug 1388013 is ready by borrowing the commits from that bug)
You need to log in
before you can comment on or make changes to this bug.
Description
•