- Notifications
You must be signed in to change notification settings - Fork95
chore: require Python ≥ 3.9, protobuf ≥ 4.25.8#877
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Summary of ChangesHello@vchudnov-g, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines the project's development and testing environment by officially ending support for Python 3.7 and 3.8. It updates various configuration files and removes legacy CI setups, aligning the project with Python 3.9 as the new minimum required version, ensuring compatibility with modern Python features and dependencies. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on ourdocumentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on@gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign uphere. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with theGemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request correctly removes support for Python 3.7 and 3.8 by updating configuration files, dependencies, and CI scripts. The changes are clean and directly address the goal of the PR. I have one suggestion intests/asyncio/test_operation_async.py to improve the correctness of a test that was modified.
Uh oh!
There was an error while loading.Please reload this page.
This is in response to a code review comment by Gemini Code Assist
vchudnov-g commentedDec 5, 2025
/gemini review |
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.
Code Review
This pull request correctly removes support for Python 3.7 and 3.8. The changes are consistent across the codebase, including updating CI configurations, Python version specifiers inpyproject.toml andnoxfile.py, and removing dependencies that were only necessary for the older Python versions. The modifications are well-executed and align with the PR's goal. As a follow-up, you could also remove theimportlib_metadata dependency frompyproject.toml (lines 53-55), as it's a backport for Python versions older than 3.8 and is no longer needed.
af514f3 tofb410f4Comparevchudnov-g commentedDec 6, 2025
Marking this ready for review. Do not merge until we have an analogous PR ready for Still trying to figure out why the test coverage went down in areas that should not have been affected. I welcome any ideas. |
| # https://stackoverflow.com/a/60605919/101923 | ||
| # | ||
| # Literal[_DEFAULT_VALUE] is an alternative, but only added in Python 3.8. | ||
| # https://docs.python.org/3/library/typing.html#typing.Literal |
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.
it looks like this comment is saying an improvement is available to us now that we're dropping 3.7. Is that something we'd want to consider, or should we stick with the existing implementation?
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.
I looked at this and it didn't seem like a cleaner alternative. I'm not sure what I had in mind when I first wrote this comment, but I think the current implementation is clean.
| # Remove once https://github.com/grpc/grpc/issues/35086 is fixed (and version newer than 1.60.0 is published) | ||
| "ignore:There is no current event loop:DeprecationWarning", | ||
| # Remove after support for Python 3.7 is dropped | ||
| "ignore:After January 1, 2024, new releases of this library will drop support for Python 3.7:DeprecationWarning", |
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.
maybe we should keep in a generic version of this warning, since we expect to start seeing it for other versions at some point in the future
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.
All the warnings issued by_python_version_support.py are of typeFutureWarning, which are captured by the rule above.
This particular configuration line was brought in frompytest.ini during the migration to TOML (#736) before we had the current Python version warnings
vchudnov-g left a comment
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.
Fixed the coverage issue; it was due to the 3.9 tests coverage file. Thanks,@daniel-sanche !
PTAL
| # https://stackoverflow.com/a/60605919/101923 | ||
| # | ||
| # Literal[_DEFAULT_VALUE] is an alternative, but only added in Python 3.8. | ||
| # https://docs.python.org/3/library/typing.html#typing.Literal |
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.
I looked at this and it didn't seem like a cleaner alternative. I'm not sure what I had in mind when I first wrote this comment, but I think the current implementation is clean.
| # Remove once https://github.com/grpc/grpc/issues/35086 is fixed (and version newer than 1.60.0 is published) | ||
| "ignore:There is no current event loop:DeprecationWarning", | ||
| # Remove after support for Python 3.7 is dropped | ||
| "ignore:After January 1, 2024, new releases of this library will drop support for Python 3.7:DeprecationWarning", |
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.
All the warnings issued by_python_version_support.py are of typeFutureWarning, which are captured by the rule above.
This particular configuration line was brought in frompytest.ini during the migration to TOML (#736) before we had the current Python version warnings
daniel-sanche left a comment
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.
LGTM
Uh oh!
There was an error while loading.Please reload this page.
Require Python 3.9+
Require protobuf 4.25.8+
Corollary for test coverage:
Simplify code accordingly.
Fixes#835
This also required removing 3.7 and 3.8 as required checks in the repo config, under branch protection rules.