Sync streaming interface on responses #695
Conversation
| @@ -917,18 +979,15 @@ def raw(self): # type: ignore | |||
| """ | |||
| A byte-iterator over the raw response content. | |||
| """ | |||
| if hasattr(self, "_raw_content"): | |||
There was a problem hiding this comment.
We can drop _raw_content here since we're not special casing that anymore.
| self._raw_stream = stream | ||
| else: | ||
| self._raw_stream = ByteStream(body=content or b"") | ||
| self.read() |
There was a problem hiding this comment.
This is a little nicer now that we can use a sync .read() inside __init__ rather than having to special-case self._raw_content.
There was a problem hiding this comment.
In line with the sync strategy we envisioned for responses (i.e. sync version of .aXYZ() methods), so yup for me for now.
Leads to a tiny bit of code duplication on the client side, but at least that keeps things easy to read and manage. :-)
Happy to give an approval when this gets out of draft!
Edit: updated the PR description to reference #572 👍 (which, btw, looks out of date now).
httpx/content_streams.py
Outdated
| async for part in (): # pragma: nocover | ||
| yield b"" |
There was a problem hiding this comment.
Does this have any actual runtime consequence, or is this to please linters?
Turns out that we can make this work without tricks by dropping the async annotation…
def __aiter__(self) -> typing.AsyncIterator[bytes]:
raise RuntimeError("Attempted to call a async iterator on an sync stream.")
tests/test_content_streams.py
Outdated
| @pytest.mark.asyncio | ||
| async def test_invalid_argument(): |
There was a problem hiding this comment.
We don't need the asyncio marker here, do we? Possible simplification…
def test_invalid_argument():
with pytest.raises(TypeError):
encode(123)
|
Thanks for the great review! |
Refs #572
Add sync interfaces for streaming on responses.
Draft PR at the moment just to demonstrate how it'll look... The tests will need filling out to.
Related to this will be:
IteratorStreamincontent_streams.py..next()