Closed Bug 1428045 Opened 7 years ago Closed 7 years ago

Make flake8 linter pass under Python 3

Categories

(Tree Management :: Treeherder, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: emorley, Assigned: ghickman, Mentored)

References

Details

(Keywords: good-first-bug, Whiteboard: [lang=py])

Attachments

(1 file, 1 obsolete file)

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
Hey, I'd love to take a shot at this one!
Hey, I've gotten through about 2/3 of the flake8 errors and want to push to git. How do I request a PR?
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 :-)
Assignee: nobody → kevinmgagnon
Status: NEW → ASSIGNED
Thanks! I think I got it to work by forking the repo first. Let me know if I need to do anything differently.
Let me know if you need a hand re the comment on the PR last week :-)
Hi Kevin - do you know if you'll get a chance to update the PR soon? :-)
Flags: needinfo?(kevinmgagnon)
Assignee: kevinmgagnon → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(kevinmgagnon)
Assignee: nobody → ghickman
Attachment #8959527 - Flags: review?(emorley)
Comment on attachment 8959527 [details] 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)
Attachment #8959527 - Flags: review?(emorley)
Assignee: ghickman → cdawson
Attachment #8944561 - Attachment is obsolete: true
Assignee: cdawson → ghickman
Attachment #8959527 - Flags: review?(cdawson)
Comment on attachment 8959527 [details] Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3347 Handing this one over to Ed. Thanks Ed!
Attachment #8959527 - Flags: review?(cdawson) → review?(emorley)
Comment on attachment 8959527 [details] Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3347 r+ with the last review comment addressed :-)
Attachment #8959527 - Flags: review?(emorley) → review+
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
Many thanks :-)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: