-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Update tornado's lower bound to the earliest secure version #1461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
12de0c5 to
6c32d12
Compare
With the help of pyupgrade
With the help of pyupgrade
The latest release of the 5.x series was in 2018 and has never received a security update since then. The latest found security issue of the 6.x series is fixed in 6.5.0
6c32d12 to
a574975
Compare
|
@Nusnus 👋🏻 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR modernizes the codebase by dropping support for older Python versions (3.7 and 3.8), bumping the minimum Tornado version to 6.5.0, and applying various code style improvements including converting %-formatting to f-strings/.format(), using raw strings for regex patterns, and simplifying comprehensions.
Key Changes
- Bumps minimum Tornado version from 5.0.0 to 6.5.0 and removes testing for Tornado versions 6.0-6.4
- Removes Python 3.7 and 3.8 from setup.py classifiers and CI matrix
- Modernizes code style: f-strings, dict/set literals, raw strings, and exception handling
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tox.ini | Reduces tornado version test matrix to only 6.5 |
| setup.py | Removes Python 3.7 and 3.8 version classifiers |
| requirements/default.txt | Bumps minimum tornado version to 6.5.0 |
| .github/workflows/build.yml | Updates CI to test Python 3.9-3.12 with Tornado 6.5 |
| tests/unit/views/test_auth.py | Uses raw string literal for regex pattern |
| tests/unit/test_command.py | Modernizes to use byte literals and f-strings |
| flower/views/workers.py | Simplifies dict comprehension syntax |
| flower/views/init.py | Uses set literals instead of set([]) |
| flower/utils/search.py | Attempts to convert list comprehension to generator (contains bug) |
| flower/utils/broker.py | Replaces deprecated socket.error and IOError with OSError |
| flower/utils/init.py | Converts %-formatting to .format() |
| flower/command.py | Replaces deprecated IOError with OSError |
| examples/tasks.py | Converts %-formatting to f-string |
| docs/conf.py | Removes Python 2 encoding declaration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {py38,py39,py310,py311}-celery52-tornado65, | ||
| # Celery 5.3: py38–py312 | ||
| {py38,py39,py310,py311,py312}-celery53-{tornado60,tornado61,tornado62,tornado63,tornado64,tornado65}, | ||
| {py38,py39,py310,py311,py312}-celery53-tornado65, | ||
| # Celery 5.4: py38–py312 | ||
| {py38,py39,py310,py311,py312}-celery54-{tornado60,tornado61,tornado62,tornado63,tornado64,tornado65}, | ||
| {py38,py39,py310,py311,py312}-celery54-tornado65, | ||
| # Celery 5.5: py38–py312 | ||
| {py38,py39,py310,py311,py312}-celery55-{tornado60,tornado61,tornado62,tornado63,tornado64,tornado65}, | ||
| {py38,py39,py310,py311,py312}-celery55-tornado65, |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tox.ini still includes Python 3.8 in the test matrix (py38), but setup.py removed the Python 3.8 classifier. This creates an inconsistency - if Python 3.8 support is being dropped, it should also be removed from the tox test matrix. If Python 3.8 is still supported, the classifier should remain in setup.py.
| fail-fast: false | ||
| matrix: | ||
| python-version: ['3.8', '3.9', '3.10', '3.11', '3.12'] | ||
| python-version: ['3.9', '3.10', '3.11', '3.12'] |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI matrix removes Python 3.8 but setup.py classifiers also remove it while tox.ini still includes py38. For consistency, if Python 3.8 support is being dropped completely, it should be removed from tox.ini as well, or if it's being kept, it should remain in setup.py classifiers and the CI matrix.
| python-version: ['3.9', '3.10', '3.11', '3.12'] | |
| python-version: ['3.8', '3.9', '3.10', '3.11', '3.12'] |
| parsed_search['kwargs'] = {} | ||
| try: | ||
| key, value = [p.strip() for p in query_part[len('kwargs:'):].split('=')] | ||
| key, value = (p.strip() for p in query_part[len('kwargs:'):].split('=')) |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change from a list comprehension to a generator expression will break the unpacking operation. Python cannot unpack a generator directly with tuple unpacking syntax. The original [p.strip() for p in ...] returns a list that can be unpacked, but (p.strip() for p in ...) returns a generator object that cannot be unpacked this way.
To fix this while still using a generator-like approach, you should use: key, value = query_part[len('kwargs:'):].split('=') and then apply .strip() to each variable individually, or keep the list comprehension.
| key, value = (p.strip() for p in query_part[len('kwargs:'):].split('=')) | |
| key, value = [p.strip() for p in query_part[len('kwargs:'):].split('=')] |
| Programming Language :: Python :: 3 :: Only | ||
| Programming Language :: Python :: 3.7 | ||
| Programming Language :: Python :: 3.8 | ||
| Programming Language :: Python :: 3.9 | ||
| Programming Language :: Python :: 3.10 | ||
| Programming Language :: Python :: 3.11 |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The python_requires field in setup.py still specifies ">=3.7", but the PR removes Python 3.7 and 3.8 from the classifiers and the tox.ini still tests Python 3.8. This creates an inconsistency - either python_requires should be updated to ">=3.8" (if Python 3.8 is the new minimum as tested in tox.ini) or ">=3.9" (if dropping 3.8 support as indicated by the removed classifiers).
The latest release of the 5.x series was in 2018 and has never received a security update since then. The latest found security issue of the 6.x series is fixed in 6.5.0
Resolves:
Depends on:
I'll rebase the branch when #1463 is merged.