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

Incorrect typing on AsyncResponse.iter_content() #185

Closed
judilsteve opened this issue Nov 28, 2024 · 1 comment
Closed

Incorrect typing on AsyncResponse.iter_content() #185

judilsteve opened this issue Nov 28, 2024 · 1 comment
Labels
typing static typing related (mypy or similar)

Comments

@judilsteve
Copy link

judilsteve commented Nov 28, 2024

One must run async for chunk in await async_response.iter_content() (which conflicts with the type hints provided for AsyncResponse.iter_content()) instead of the expected async for chunk in async_response.iter_content().

Expected Result

Content of the HTTP response to be printed in chunks to stdout.

Actual Result

Exception:

Traceback (most recent call last):
  File "<redacted>/test.py", line 10, in <module>
    if __name__ == '__main__': asyncio.run(_main())
                               ^^^^^^^^^^^^^^^^^^^^
  File "/home/james/.pyenv/versions/3.12.1/lib/python3.12/asyncio/runners.py", line 194, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/home/james/.pyenv/versions/3.12.1/lib/python3.12/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/james/.pyenv/versions/3.12.1/lib/python3.12/asyncio/base_events.py", line 684, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "<redacted>/test.py", line 7, in _main
    async for chunk in resp.iter_content():
TypeError: 'async for' requires an object with __aiter__ method, got coroutine

Reproduction Steps

import asyncio
from niquests import AsyncSession

async def _main():
    async with AsyncSession() as session:
        resp = await session.get('https://github.com/jawah/niquests', stream=True)
        async for chunk in resp.iter_content():
            print(chunk)

if __name__ == '__main__': asyncio.run(_main())

System Information

$ python -m niquests.help
{
  "charset_normalizer": {
    "version": "3.4.0"
  },
  "http1": {
    "h11": "0.14.0"
  },
  "http2": {
    "jh2": "5.0.4"
  },
  "http3": {
    "enabled": true,
    "qh3": "1.2.1"
  },
  "idna": {
    "version": "3.10"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.12.1"
  },
  "niquests": {
    "version": "3.11.1"
  },
  "ocsp": {
    "enabled": true
  },
  "platform": {
    "release": "5.15.153.1-microsoft-standard-WSL2",
    "system": "Linux"
  },
  "system_ssl": {
    "version": "300000d0"
  },
  "urllib3.future": {
    "cohabitation_version": null,
    "version": "2.12.900"
  },
  "wassima": {
    "certifi_fallback": false,
    "enabled": true,
    "version": "1.1.5"
  }
}

Workaround

import asyncio
from niquests import AsyncSession
from typing import cast, Coroutine, AsyncGenerator

async def _main():
    async with AsyncSession() as session:
        resp = await session.get('https://github.com/jawah/niquests', stream=True)
        async for chunk in await cast(Coroutine[None, None, AsyncGenerator[bytes, None]], resp.iter_content()):
            print(chunk)

if __name__ == '__main__': asyncio.run(_main())
@Ousret
Copy link
Member

Ousret commented Dec 5, 2024

It's true, something isn't right. But it is not what you think it is.
I suspect there's a bug (some edge case) that bring this bad UX. It does not seems to be something we can deal with within Niquests.

If you have a (rather clean, without breakage) solution that we can apply that makes both modern IDEs and mypy happy with, we'll proceed.

async def reproducer() -> typing.AsyncGenerator[bytes, None]:
    async def gen() -> typing.AsyncGenerator[bytes, None]:
        yield b"foo"
        yield b"bar"
    return gen()

So there you have to await reproducer() as their is no yield within reproducer. This minimal code does not confuse mypy or IDEs. But Niquests makes it glitch somehow, making it "okay" to not await the method.

We'll need a typing expert to investigate.

Regards,

@Ousret Ousret added help wanted Extra attention is needed typing static typing related (mypy or similar) labels Dec 6, 2024
@Ousret Ousret mentioned this issue Dec 13, 2024
@Ousret Ousret removed the help wanted Extra attention is needed label Dec 13, 2024
@Ousret Ousret closed this as completed in 6021f13 Dec 13, 2024
Ousret added a commit that referenced this issue Dec 13, 2024
3.11.3 (2024-12-13)
-------------------

**Fixed**
- Static type checker getting confused around ``AsyncSession`` and
attached overloads (``AsyncResponse`` or ``Response``). (#185)

**Changed**
- Default keepalive (HTTP/2, and HTTP/3) changed to 1 hour. In
conformance with urllib3-future.

**Removed**
- Automatic resolution of pending lazy responses if there are too many
of them.
Previously, we hardcoded a limit of 128 * NUM_CONN_POOL maximum inflight
(aka. unresolved/lazy) response.
This was unrealistic due to a number of factors like (but not limited
to):
  A) remote peers can choose at will the max streams.
  B) we can have multiple pool with multiple (varying) max capacities.
C) retrieving max streams per pool per conn is a very costly procedure
(in terms of performance).
We will revisit this later on. You still can set
``max_in_flight_multiplexed`` in your ``HTTPAdapter`` to
  restore this broken behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typing static typing related (mypy or similar)
Projects
None yet
Development

No branches or pull requests

2 participants