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

Drop .read/.aread from SyncByteStream/AsyncByteStream #2407

Merged
merged 3 commits into from Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 2 additions & 6 deletions httpx/_content.py
Expand Up @@ -52,9 +52,7 @@ def __iter__(self) -> Iterator[bytes]:
raise StreamConsumed()

self._is_stream_consumed = True
if hasattr(self._stream, "read") and not isinstance(
self._stream, SyncByteStream
):
Copy link
Member Author

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.

if hasattr(self._stream, "read"):
# File-like interfaces should use 'read' directly.
chunk = self._stream.read(self.CHUNK_SIZE) # type: ignore
while chunk:
Expand All @@ -79,9 +77,7 @@ async def __aiter__(self) -> AsyncIterator[bytes]:
raise StreamConsumed()

self._is_stream_consumed = True
if hasattr(self._stream, "aread") and not isinstance(
self._stream, AsyncByteStream
):
if hasattr(self._stream, "aread"):
# File-like interfaces should use 'aread' directly.
chunk = await self._stream.aread(self.CHUNK_SIZE) # type: ignore
while chunk:
Expand Down
32 changes: 0 additions & 32 deletions httpx/_types.py
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an example from the old-style Transport API.

We had .read() so that users could eg. stream.read() here.

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:
Expand All @@ -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()
4 changes: 2 additions & 2 deletions tests/test_content.py
Expand Up @@ -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()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're matching the style of the other test cases now.
These .read()/.aread() operations were only added to this test case so that we'd have coverage of those code lines.

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""
Expand Down