-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Use per-file-target-version to define Python 3.9 for CLI code #12362
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
Conversation
3fa2f58
to
b641829
Compare
# Only allow minimum version for code used in the CLI | ||
"localstack-core/localstack/cli/**" = "py39" | ||
"localstack-core/localstack/packages/**" = "py39" | ||
"localstack-core/localstack/config.py" = "py39" | ||
"localstack-core/localstack/constants.py" = "py39" | ||
"localstack-core/localstack/utils/analytics/**" = "py39" | ||
"localstack-core/localstack/utils/bootstrap.py" = "py39" | ||
"localstack-core/localstack/utils/json.py" = "py39" |
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'm unsure whether this set of files is correct or maybe even too much (e.g., in the case of the analytics code where I even had to remove a match
statement)
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 think this is good for now! we can always remove files from here
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! thanks for cleaning up 🚀
# Only allow minimum version for code used in the CLI | ||
"localstack-core/localstack/cli/**" = "py39" | ||
"localstack-core/localstack/packages/**" = "py39" | ||
"localstack-core/localstack/config.py" = "py39" | ||
"localstack-core/localstack/constants.py" = "py39" | ||
"localstack-core/localstack/utils/analytics/**" = "py39" | ||
"localstack-core/localstack/utils/bootstrap.py" = "py39" | ||
"localstack-core/localstack/utils/json.py" = "py39" |
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 think this is good for now! we can always remove files from here
Motivation
As in dependent repositories, we have an issue with the ASF updates since the latest
ruff
update. The definition of the Python 3.9 target version in linting/formatting leads to syntax errors when enabling preview features like we do in the ASF update action.To fix that, we need to define our versions correctly depending on if the code runs in the CLI or in the runtime (container)
Changes
py311
matching our.python-version
per-file-target-version
topy39
for all code accessed in the CLI matching our minimum required versionpy311
py39
compatible (remove match statement fromusage.py
)