Skip to content

Add support for Python 3.12 and enable it in CI #54

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jul 12, 2024
Merged

Add support for Python 3.12 and enable it in CI #54

merged 5 commits into from
Jul 12, 2024

Conversation

mkmkme
Copy link
Collaborator

@mkmkme mkmkme commented Jul 12, 2024 •
edited
Loading

Pull Request check-list

Please make sure to review and check all of these items:

  • Do tests and lints pass with this change?
  • Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Description of change

This changeset finishes adding support for Python 3.12. This should close #24

@mkmkme
Copy link
Collaborator Author

mkmkme commented Jul 12, 2024

Oh, I didn't know updating black would introduce syntax errors for 3.8.

I'm going to drop this commit then. Not a biggie.

@codecov-commenter
Copy link

codecov-commenter commented Jul 12, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 75.01%. Comparing base (e4cb9e2) to head (631a337).

Files Patch % Lines
valkey/commands/timeseries/info.py 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #54      +/-   ##
==========================================
- Coverage   75.01%   75.01%   -0.01%     
==========================================
  Files         132      132              
  Lines       34368    34367       -1     
==========================================
- Hits        25781    25780       -1     
  Misses       8587     8587

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ahmedsobeh
Copy link
Collaborator

Oh, I didn't know updating black would introduce syntax errors for 3.8.

I'm going to drop this commit then. Not a biggie.

yeah I faced this is another PR too. I think the line size limit on Black is different from whatever checks the linting in CI

@mkmkme
Copy link
Collaborator Author

mkmkme commented Jul 12, 2024

I think the line size limit on Black is different from whatever checks the linting in CI

For me it wasn't even about the line size. Apparently adding parenthesis to with statement and moving its internals to different lines wasn't valid in 3.8. Bizarre!

aiven-sal
aiven-sal reviewed Jul 12, 2024
View reviewed changes
mkmkme added 5 commits July 12, 2024 11:29
@mkmkme
fix E721 across the codebase
6cb20cc 
`flake8`s E721 rule states that the types should not be compared.
Instead, `isinstance` should be used.

This commit replaces all the type comparisons with calls for
`isinstance`.

[1] https://www.flake8rules.com/rules/E721.html

Signed-off-by: Mikhail Koviazin <mikhail.koviazin@aiven.io>
@mkmkme
commands: remove redundant definition in core.py
2853ac9 
`AsyncManagementCommands` was first defined as `ManagementCommands` and
just two lines below it was redefined as a class derived from
`ManagementCommands`. This triggered F811 error in flake8 [1].
This commit removes the redundant definition.

[1]: https://www.flake8rules.com/rules/F811.html

Signed-off-by: Mikhail Koviazin <mikhail.koviazin@aiven.io>
@mkmkme
Loosen dev_requirements.txt
04d0a48 
dev_requirements.txt used to have pinned version of some of dev
dependencies. Some of these pinned versions didn't work with Python3.12
at all.

This commit removes most of pinned versions for now.

Signed-off-by: Mikhail Koviazin <mikhail.koviazin@aiven.io>
@mkmkme
tasks: add option to force colors
6111059 
passing `-c/--color` to `invoke` will enable colored output in the
tasks. This argument must go _after_ the task name. Examples:

	inv tests -c
	invoke linters --color

This commit fixes #47

Signed-off-by: Mikhail Koviazin <mikhail.koviazin@aiven.io>
@mkmkme
github: enable Python 3.12 in CI
631a337 
Signed-off-by: Mikhail Koviazin <mikhail.koviazin@aiven.io>
@mkmkme mkmkme requested a review from aiven-sal July 12, 2024 12:44
@aiven-sal aiven-sal merged commit 1901d72 into valkey-io:main Jul 12, 2024
52 checks passed
@kjaymiller
Copy link
Contributor

So excited to see this merged!

@mkmkme mkmkme deleted the mkmkme/python312 branch July 12, 2024 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aiven-sal aiven-sal aiven-sal approved these changes

@ahmedsobeh ahmedsobeh Awaiting requested review from ahmedsobeh ahmedsobeh is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add development support for python 3.12
5 participants
@mkmkme @codecov-commenter @ahmedsobeh @kjaymiller @aiven-sal