Instance of `StreamingBody` returns `True` for `isinstance` `Iterable` check

FeeeeK commented 5 months ago

Describe the bug StreamingBody returns True for isinstance Iterable check when it shouldn't, this can lead to some problems, for example, wrong content encoding in httpx (link) and inability to use StreamingBody as content in httpx.AsyncClient

> isinstance(StreamingBody, Iterable)
False
> isinstance(StreamingBody, AsyncIterable)
False
> isinstance(StreamingBody(b"", 0), Iterable)
True
> isinstance(StreamingBody(b"", 0), AsyncIterable)
True
> StreamingBody(b"", 0).__iter__
<method-wrapper '__iter__' of StreamingBody object at 0x0000021D9F2B93C0>
> StreamingBody(b"", 0).__aiter__
<bound method StreamingBody.__aiter__ of <StreamingBody at 0x0000021D9FC6AC40 for bytes at 0x00007FF96586D328>>
> for _ in streaming_body: pass
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: 'ClientResponse' object is not iterable

Checklist

[x] I have reproduced in environment where pip check passes without errors
[x] I have provided pip freeze results
[x] I have provided sample code or detailed way to reproduce
[x] I have tried the same code in botocore to ensure this is an aiobotocore specific issue
[x] I have tried similar code in aiohttp to ensure this is is an aiobotocore specific issue
[x] I have checked the latest and older versions of aiobotocore/aiohttp/python to see if this is a regression / injection

pip freeze results

...
aioboto3==11.3.1
aiobotocore==2.6.0
aiohttp==3.9.3
boto3==1.28.17
botocore==1.31.17
...

Environment:

Python Version: 3.11.8
OS name and version: windows

Additional context Add any other context about the problem here.

jakob-keller commented 5 months ago

Hello and thanks for reporting!

I believe you are correct: StreamingBody is indeed not iterable. The false positive isinstance check appears to be due to having wrapt.ObjectProxy as a parent class.

There might be several ways to deal with this issue:

Make wrapt.ObjectProxy play nicely with isinstance checks by addressing the issue upstream
Tweak StreamingBody to ensure isinstance check returns expected result (if possible)
Drop wrapt.ObjectProxy as a parent class
Leave StreamingBody as it is and work around the issue in downstream code

@thehesiod: Any thoughts?

FeeeeK commented 5 months ago

Looks like this issue is already 8 years old: https://github.com/GrahamDumpleton/wrapt/issues/93 so it seems to me that option 3 would most likely be the solution

jakob-keller commented 5 months ago

Looks like this issue is already 8 years old: GrahamDumpleton/wrapt#93 so it seems to me that option 3 would most likely be the solution

That's what I feared

thehesiod commented 5 months ago

nice detective work. we can drop the proxy, just means we'll have to be more careful when upgrading

On Mon, Mar 11, 2024, 7:07 AM Jakob Keller @.***> wrote:

Looks like this issue is already 8 years old: GrahamDumpleton/wrapt#93 https://github.com/GrahamDumpleton/wrapt/issues/93 so it seems to me that option 3 would most likely be the solution

That's what I feared

— Reply to this email directly, view it on GitHub https://github.com/aio-libs/aiobotocore/issues/1098#issuecomment-1988529691, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6Q77PQJ24X5OMKDFBQCH3YXW3DRAVCNFSM6AAAAABEQI7O6WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBYGUZDSNRZGE . You are receiving this because you were mentioned.Message ID: @.***>

thehesiod commented 1 month ago

I'll try to get a fix for this out soon, was hoping someone would come with a pr before me :)

aio-libs / aiobotocore

Instance of `StreamingBody` returns `True` for `isinstance` `Iterable` check #1098