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

valkey-io / valkey-py

Valkey Python client based on a fork of redis-py

MIT License

71 stars 9 forks source link

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

Closed ali-gholami-aiven closed 2 months ago

ali-gholami-aiven commented 2 months ago

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

codecov-commenter commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

Additional details and impacted files

```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nirav-ark-biotech commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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