-
-
Notifications
You must be signed in to change notification settings - Fork 857
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
Drop .read
/.aread
from SyncByteStream
/AsyncByteStream
#2407
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,33 +107,7 @@ def close(self) -> None: | |
""" | ||
Subclasses can override this method to release any network resources | ||
after a request/response cycle is complete. | ||
|
||
Streaming cases should use a `try...finally` block to ensure that | ||
the stream `close()` method is always called. | ||
|
||
Example: | ||
|
||
status_code, headers, stream, extensions = transport.handle_request(...) | ||
try: | ||
... | ||
finally: | ||
stream.close() | ||
Comment on lines
-116
to
-120
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an example from the old-style Transport API. We had The Transport API now just returns fully fledged responses, so this is redundant. |
||
""" | ||
|
||
def read(self) -> bytes: | ||
""" | ||
Simple cases can use `.read()` as a convenience method for consuming | ||
the entire stream and then closing it. | ||
|
||
Example: | ||
|
||
status_code, headers, stream, extensions = transport.handle_request(...) | ||
body = stream.read() | ||
""" | ||
try: | ||
return b"".join([part for part in self]) | ||
finally: | ||
self.close() | ||
|
||
|
||
class AsyncByteStream: | ||
|
@@ -145,9 +119,3 @@ async def __aiter__(self) -> AsyncIterator[bytes]: | |
|
||
async def aclose(self) -> None: | ||
pass | ||
|
||
async def aread(self) -> bytes: | ||
try: | ||
return b"".join([part async for part in self]) | ||
finally: | ||
await self.aclose() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,8 +13,8 @@ async def test_empty_content(): | |
assert isinstance(stream, httpx.SyncByteStream) | ||
assert isinstance(stream, httpx.AsyncByteStream) | ||
|
||
sync_content = stream.read() | ||
async_content = await stream.aread() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're matching the style of the other test cases now. |
||
sync_content = b"".join([part for part in stream]) | ||
async_content = b"".join([part async for part in stream]) | ||
|
||
assert headers == {} | ||
assert sync_content == b"" | ||
|
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.
The
and not isinstance(self._stream, SyncByteStream)
is what prompted me to clean these up.It's such an oddly confusing branch - I couldn't figure out why it existed.
"Treat file-like objects as file-like objects, except for this one specific case".
No thanks.