-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
ci: make Python 3.9 minimum required build #13066
Conversation
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.
Hi @jackwotherspoon and reviewers,
I'm Gemini, and I'm currently reviewing this pull request. In the meantime, here's a summary to help everyone quickly understand the changes:
This pull request, "chore: make Python 3.9 minimum required build", aims to update the minimum required Python version for the project builds to 3.9. The rationale provided is that Python 3.8 has reached its end-of-life.
The primary change is within the .github/sync-repo-settings.yaml
file. Specifically, line 47 of this file was modified to update the required Kokoro CI build for Python from 3.8 to 3.9. Before the change, line 47 read:
47: - "Kokoro CI - Python 3.8"
After the change, line 47 now reads:
47: - "Kokoro CI - Python 3.9"
This ensures that builds will now fail if they don't meet the updated minimum Python version requirement.
I'll have a full review shortly.
Here's a little haiku to brighten your day:
Old Python's now gone,
Three nine takes its rightful place,
Code runs, fresh and new.
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 change looks good. Python 3.8 reached end-of-life on 2024-10-14, so it's good to remove it from the required builds. The PR body correctly links to the Python devguide for EOL information.
One suggestion is to update the nox -s py-3.9
command in .github/PULL_REQUEST_TEMPLATE.md
to reflect the new minimum Python version. This will help contributors run the correct tests locally before submitting PRs. Also, the Python versions listed in AUTHORING_GUIDE.md
should be updated.
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.
there are probably other areas that need to get fixed by this change because I think there are configurations that say ignore Python 3.9 testing.
@iennae Do you have any examples? I can update them if would you like as part of this PR if need be. |
@iennae that's a great point. I guess we could grep over noxfile_config files to look for ignored versions including 3.9, and then update them accordingly |
@davidcavazos @iennae I went through all the Let me know if that approach seems okay to you 👍 |
@@ -923,7 +923,7 @@ Add the new environment variables to the `envs` dictionary. | |||
```py | |||
TEST_CONFIG_OVERRIDE = { | |||
# You can opt out from the test for specific Python versions. | |||
"ignored_versions": ["2.7", "3.7", "3.9", "3.10", "3.11"], | |||
"ignored_versions": ["2.7", "3.8", "3.10", "3.11", "3.12"], |
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.
did this need to match 3.13 from above?
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.
Don't believe so, we want samples to support all those versions but we purely test lowest and highest supported version by default. Does that make sense?
@@ -44,7 +44,7 @@ branchProtectionRules: | |||
requiredStatusCheckContexts: | |||
- "Kokoro CI - Lint" | |||
- "Kokoro CI - Python 2.7 (App Engine Standard Only)" |
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.
can we remove the Python 2.7 requirement -- it's no longer deployable to that runtime and the existing samples are meant as reference/migration versus something that can be actively tested (other than fixing old region tags to ensure we can track them there won't be modifications).
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'll do that as a follow-up PR, want to make this one purely Python 3.8 --> Python 3.9
Python 3.8 is EOL: https://devguide.python.org/versions/
Making Python 3.9 the minimum required build to pass.