aio-libs / aiohappyeyeballs

Happy Eyeballs for pre-resolved hosts
Other
11 stars 9 forks source link

Support Python 3.8+ in backported asyncio.staggered #96

Closed vemel closed 1 month ago

vemel commented 1 month ago

What do these changes do?

Even though backported staggered.py for Python 3.12+ is used only in Python3.12, this module cannot be imported in Python 3.8. I backported an older version of staggered.py that is compatible with Python 3.8, but does not support eager task factories. I preserved type annotations and added missing ones.

I think, since aiohappyeyeballs is a py38+ project, all modules should be at least importable in py38+ for linting and type checking. Also, staggered_race function signature should now be the same for py312 and for lower python versions.

Backported from https://github.com/python/cpython/blob/16b46ebd2b0025aa461fdfc95fbf98a4f04b49e6/Lib/asyncio/staggered.py, added missing type annotations.

Are there changes in behavior for the user?

No

Related issue number

No related issue number

Checklist

Dreamsorcerer commented 1 month ago

all modules should be at least importable in py38+

I'm not sure how you reached this conclusion. It's a private module that is only imported on newer versions of Python. Why would you be trying to import the private module directly?

but does not support eager task factories.

So, it breaks the main thing that we want to fix when this module is actually used...?

Dreamsorcerer commented 1 month ago

for linting and type checking

I don't think the internals of the library should affect your linting or type checking. If they do (and we'd see this in aiohttp anyway), then probably adding a typing stub or something is a more sensible fix.

vemel commented 1 month ago

Actually, the internals affect type checking:

mypy: /opt/hostedtoolcache/Python/3.10.15/x64/lib/python3.10/site-packages/aiohappyeyeballs/_staggered.py:98: error: invalid syntax; you likely need to run mypy using Python 3.12 or newer  [syntax]

This happens because in type checking mode if sys.version ... conditions are ignored, type checker uses the first import it finds.

bdraco commented 1 month ago

The simplest fix to make mypy happy would be an if TYPE_CHECKING block

vemel commented 1 month ago

I am closing this PR. Fixed on my side.

Dreamsorcerer commented 1 month ago

Yeah, seems to have passed fine on aiohttp. The only way you should see an issue like that is if you're actually type checking the library itself (which can happen when using MYPYPATH).