1428045 - Make flake8 linter pass under Python 3

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect, P3)

Description

[Ed Morley [:emorley]]

Treeherder currently runs under Python 2.7, however we wish to eventually migrate to Python 3. Some initial steps towards that have already taken place (see bug 1330474) - next on the list is making the linter 'flake8' not output any errors when run against the Treeherder codebase under Python 3 + adding Travis CI support for making sure it doesn't regress afterwards.

Suggested steps:
1) Install latest Python 3.6 and pip in your local environment (I wouldn't worry about using the Treeherder Vagrant environment for now; it uses Python 2 not 3, and you'll need it to still be on Python 2 to verify your changes don't break Python 2 compatibility).
2) Install virtualenvwrapper (https://virtualenvwrapper.readthedocs.io/en/latest/)
3) Create a Python 3 virtualenv using something like: `mkvirtualenv py3-flake8 -p python3`
4) Install flake8 into that environment using `pip install flake8`
5) Git clone https://github.com/mozilla/treeherder
6) From the root of the Treeherder repo run `flake8` with no options (it will use the settings from setup.cfg)
7) Look up what the failing rules mean on [1] and/or Google as needed.
8) Modify the affected lines, making sure to keep compatibility with Python 2. See [2] and [3] for tips on writing Python 2+3 compatible code. NB: Don't worry about making any changes other than those needed to fix the flake8 errors. The rest can wait for the next bug :-)
9) Keep re-running flake8 until it passes under Python 3.6.
10) Copy the `flake8` line from the Python 2 linter section in `.travis.yml` [4] to the Python 3 section [5], so it's run under Python 3 too.
11) Commit your work (make sure to use a branch other than `master` and copy the style of the existing commit messages - also multiple commits would probably be best), push to GitHub and open a PR. This will trigger a full test run on Travis.
12) If any of the existing tests fail on Travis under Python 2, optionally create a Vagrant environment locally [6] so you can run the full stack tests locally [7], to make debugging quicker.
13) Once the PR's tests pass, mark the attachment that will have been auto-created on this bug for review ("review?<NAME>")

If you have any questions, just ask :-)

The errors reported by flake8 at the moment under Python 3:

./lints/queuelint.py:27:58: E999 SyntaxError: invalid syntax
./tests/etl/test_perf_schema.py:26:15: E999 SyntaxError: invalid syntax
./tests/log_parser/test_tasks.py:45:19: E999 SyntaxError: invalid syntax
./tests/model/test_bugscache.py:120:21: E999 SyntaxError: invalid syntax
./tests/model/test_error_summary.py:33:12: E999 SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in position 10-11: truncated \uXXXX escape
./tests/webapp/api/test_bugzilla.py:19:25: E999 SyntaxError: invalid syntax
./tests/webapp/api/test_bug_job_map_api.py:28:21: E999 SyntaxError: invalid syntax
./tests/webapp/api/test_job_details_api.py:47:14: E211 whitespace before '('
./tests/webapp/api/test_job_details_api.py:53:19: E999 SyntaxError: invalid syntax
./treeherder/etl/artifact.py:164:38: F821 undefined name 'unicode'
./treeherder/etl/jobs.py:34:16: F821 undefined name 'long'
./treeherder/etl/perf.py:26:34: F821 undefined name 'basestring'
./treeherder/etl/text.py:7:52: E999 SyntaxError: invalid syntax
./treeherder/perf/management/commands/test_analyze_perf.py:63:17: E999 SyntaxError: invalid syntax
./treeherder/perfalert/perfalert/__init__.py:98:16: F821 undefined name 'cmp'


[1] https://flake8.readthedocs.io/en/latest/user/error-codes.html
[2] https://eev.ee/blog/2016/07/31/python-faq-how-do-i-port-to-python-3/
[3] http://python-future.org/
[4] https://github.com/mozilla/treeherder/blob/0b31c14d308a7b856c31c292102993e02124a067/.travis.yml#L22
[5] https://github.com/mozilla/treeherder/blob/0b31c14d308a7b856c31c292102993e02124a067/.travis.yml#L37
[6] https://treeherder.readthedocs.io/installation.html
[7] https://treeherder.readthedocs.io/common_tasks.html#running-the-tests

Comment 1

[Kevin Gagnon]

Hey, I'd love to take a shot at this one!

Comment 2

[Kevin Gagnon]

Hey, I've gotten through about 2/3 of the flake8 errors and want to push to git. How do I request a PR?

Comment 3

[Ed Morley [:emorley]]

Hi Kevin - that's great!

GitHub have some docs that probably do a better job of explaining than I could:
https://help.github.com/articles/about-pull-requests/
https://help.github.com/articles/creating-a-pull-request-from-a-fork/

...but let me know if you have any questions not answered in them :-)

Comment 4

[Kevin Gagnon]

Thanks! I think I got it to work by forking the repo first. Let me know if I need to do anything differently.

Comment 5

[Ed Morley [:emorley]]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3111

Comment 6

[Ed Morley [:emorley]]

Let me know if you need a hand re the comment on the PR last week :-)

Comment 7

[Ed Morley [:emorley]]

Hi Kevin - do you know if you'll get a chance to update the PR soon? :-)

Comment 8

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3347

Comment 9

[Ed Morley [:emorley]]

Comment on attachment 8959527 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3347

Thank you for the PRs - I've left some comments.
For re-review Cameron's probably best bet until I get back (enter ":camd" to autocomplete; people tend to colon-prefix their IRC nicks in the Bugzilla display name to ease auto-completion)

Comment 10

[Cameron Dawson [:camd]]

Comment on attachment 8959527 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3347

Handing this one over to Ed.  Thanks Ed!

Comment 11

[Ed Morley [:emorley]]

Comment on attachment 8959527 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3347

r+ with the last review comment addressed :-)

Comment 12

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/8aaeb81d2c24bc94f77a6c06c8665faffd51af61
Bug 1428045 - Use 2/3 compatible base types

https://github.com/mozilla/treeherder/commit/b9878845e2bf156c17254476eb83c150a6ca820b
Bug 1428045 - Replace cmp with modern ordering methods

https://github.com/mozilla/treeherder/commit/a14b4774ca6d8f253b4777cd25c538a8e21d6e9e
Bug 1428045 - Write tests for unicode filtering

https://github.com/mozilla/treeherder/commit/07c265223bbe19bdc9e4368816a0f0bf812fc4d3
Bug 1428045 - Pull apart astral_filter's lambda expression

This is just a reimplementation of the lambda as a function to aid
readability and documentation.

https://github.com/mozilla/treeherder/commit/f7b91ca729a0f956e90551c5556a50aab80d68a2
Bug 1428045 - Conditionally set the filter based on python version

Unicode raw string literals are a syntax error in Python 3 and there
appears to be no nicer way to handle this for both versions.

https://github.com/mozilla/treeherder/commit/52ea16afb74ffc70e99aa980af97128dfbc68cae
Bug 1428045 - Ignore python 2 only syntax

We can't have a single solution for both versions so best to not lint
for this particular error (SyntaxError) here.

https://github.com/mozilla/treeherder/commit/9cdeb4665a73a3b100fcfe6c2f2e3e96705808b2
Bug 1428045 - Convert print statments to print functions

https://github.com/mozilla/treeherder/commit/f7d97026a932c7a48fba068b636a7bec20765a95
Bug 1428045 - Mark string as raw

This stops occurrences of `\u` (such as in Windows paths) being
interpreted as malformed Unicode points.

https://github.com/mozilla/treeherder/commit/29314ad6244ea741e4aca67e12722edbd7d350e2
Bug 1428045 - Run flake8 under Python 3 in CI

https://github.com/mozilla/treeherder/commit/37bd9fe649f668dc390c553382cb62b47e0f1163
Bug 1428045 - Add future imports to turn print statements into functions

https://github.com/mozilla/treeherder/commit/95420b01f42fd3db7e4a1e40b6147727ec2dc247
Bug 1428045 - Flatten conditional to only check for strings

Comment 13

[Ed Morley [:emorley]]

Many thanks :-)