Skip to content
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

update python version constraint to < 3.11 for async-timeout requirement #87

Conversation

ali-gholami-aiven
Copy link
Contributor

async-timeout library has effectively been upstreamed into Python 3.11+. installing it for python >= 3.11 should not be necessary.

this will enable packaging valkey-py for fedora 39, which comes with python 3.12 and does not conatin async-timeout 4.0.3 package.

see the deprecation notice:
https://github.com/aio-libs/async-timeout?tab=readme-ov-file#deprecated

Pull Request check-list

  • 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)?

Description of change

@ali-gholami-aiven ali-gholami-aiven force-pushed the fix-async-timeout-python-requirement branch from 1d18b16 to d7962f0 Compare September 8, 2024 12:35
@codecov-commenter
Copy link

codecov-commenter commented Sep 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.12%. Comparing base (1c6ce95) to head (a315de8).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #87      +/-   ##
==========================================
- Coverage   75.12%   75.12%   -0.01%     
==========================================
  Files         132      132              
  Lines       34398    34398              
==========================================
- Hits        25841    25840       -1     
- Misses       8557     8558       +1     

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

@ali-gholami-aiven ali-gholami-aiven marked this pull request as ready for review September 9, 2024 17:40
Copy link
Collaborator

@mkmkme mkmkme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

@aiven-sal aiven-sal enabled auto-merge September 10, 2024 07:57
async-timeout library has effectively been upstreamed into Python 3.11+.
installing it for python >= 3.11 should not be necessary.

this will enable packaging valkey-py for fedora 39, which
comes with python 3.12 and does not conatin async-timeout 4.0.3 package.

see the deprecation notice:
https://github.com/aio-libs/async-timeout?tab=readme-ov-file#deprecated

Signed-off-by: Ali Gholami <ali.gholami@aiven.io>
@aiven-sal aiven-sal force-pushed the fix-async-timeout-python-requirement branch from d7962f0 to a315de8 Compare September 10, 2024 07:57
@aiven-sal aiven-sal merged commit 56766a1 into valkey-io:main Sep 10, 2024
75 checks passed
@nirav-ark-biotech
Copy link
Contributor

Hi @ali-gholami-aiven @aiven-sal quick question about this one

are we saying that, in this PR, v6.0.1 is no longer usable with Python 3.11.0? That is what I'm on rn, and bc of this block in valkey/asyncio/connection.py

if sys.version_info >= (3, 11, 3):
    from asyncio import timeout as async_timeout
else:
    from async_timeout import timeout as async_timeout

this code now fails by trying to import an async_timeout that doesn't exist. Which is fine, I can upgrade all my systems up in Python version but just wanted to get a recommendation here

@mkmkme
Copy link
Collaborator

mkmkme commented Sep 10, 2024

Hey @nirav-ark-biotech,

Hmm that sounds odd. It should be usable still.
Does it help if you do pip install async_timeout first?

EDIT: Sorry I probably misunderstood the problem here. Could you run the python RESP and try to execute

from asyncio import timeout as async_timeout

? Will this work for you?

@nirav-ark-biotech
Copy link
Contributor

nirav-ark-biotech commented Sep 10, 2024

yeah that works I'll just pip install async timeout first, which fixes it. maybe all this codebase needs is a documentation update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants