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)

defect

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.
[1] Note the 'promise' package may be required until https://github.com/syrusakbary/promise/issues/40 is fixed.
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).
(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.
Hello, this would be the second bug I'd be working on. Can you assign this to me?
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
Attached patch bug1389592.patch (obsolete) — Splinter Review
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+
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)
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+
(In reply to Autolander from comment #9)
> Created attachment 8899216 [details] [review]
> [treeherder] Dyex719:patch-1 > mozilla:master

Please take a look at this.
Status: ASSIGNED → NEW
Flags: needinfo?(emorley)
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)
Attachment #8902742 - Flags: review?(emorley)
Comment on attachment 8902742 [details] [review]
[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)
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)
Comment on attachment 8899216 [details] [review]
[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+
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)
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Yep I'm interested! Thanks for your help! I'll look into the new steps soon :)
Can you guide me towards another bug? Should I work on https://bugzilla.mozilla.org/show_bug.cgi?id=1388018 or something else?
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.

Attachment

General

Created:
Updated:
Size: