-
-
Notifications
You must be signed in to change notification settings - Fork 858
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
Support Response(content=<bytes iterator>)
#1265
Conversation
I'm going to merge this one in, since I don't think it's all that controversial, and it's blocking some other work from progressing at the moment. (Update: see #1281) I think it makes sense to use the merge-without-a-review admin privileges occasionally, so long as it's treated carefully like this (eg. controversial PRs, that have had a few days breathing room), since practically I've got a lot more time available to this right now than the rest of the maintenance team, so we're getting a bit bottle necked. (But happy to talk about any of that if needed.) |
elif hasattr(content, "__aiter__"): | ||
content = typing.cast(typing.AsyncIterator[bytes], content) | ||
return AsyncIteratorStream(aiterator=content) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you were already aware, but a thing I just learned from reviewing encode/starlette#1041: collections.abc.AsyncIterator
/collections.abc.Iterator
(which the typing
equivalents derive from) have a subclasshook which I think means you could just do:
elif isinstance(content, typing.AsyncIterator):
return AsyncIteratorStream(aiterator=content)
Also technically you may be casting Iterables to Iterators here by just checking for the presence of __aiter__
, though it seems unlikely to cause problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refs #1253
Support
Response(content=<bytes iterator>)
andResponse(content=<bytes aiterator>)
.Related follow ups will be...
Response(content=<byte stream>)
Request(content=<bytes|bytes iterator|bytes aiterator|byte stream)
Response(stream=...)
andRequest(stream=...)
A good motivation when considering this pull request in isolation, is to look over the test cases that were previously importing from the private API -
from httpx._content_streams import AsyncIteratorStream
andfrom httpx._content_streams import IteratorStream
For instance we've got cases that currently look like this...
After this pull request they get slimmed down to a more natural looking API...