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

Turn transport.request() into a context manager #206

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
cc27a65
Turn transport.request() into a context manager
florimondmanca Oct 3, 2020
dcf015e
Fix Python 3.6 compatibility
florimondmanca Oct 3, 2020
4530efe
Sync docs index snippets
florimondmanca Oct 3, 2020
6be7698
Tweak call order of _response_closed
florimondmanca Oct 3, 2020
3dfe78e
Merge branch 'master' into fm/request-context-manager
florimondmanca Oct 8, 2020
35e006a
Lint
florimondmanca Oct 8, 2020
793f285
Drop exitstack from connection_pool implementation
tomchristie Nov 16, 2020
aa99f4b
Drop ResponseStream.close in http11 implementation
tomchristie Nov 16, 2020
02de08a
Drop ResponseStream.close in http2 implementation
tomchristie Nov 16, 2020
e7612a5
Merge master
tomchristie Nov 16, 2020
fdccfaf
Resolve typo in README
tomchristie Nov 16, 2020
9d7885e
Ensure response_closed is called
tomchristie Nov 17, 2020
a92a82d
Drop close on bytestream interface
tomchristie Nov 18, 2020
3699f46
Drop bytestream.close in docs API reference
tomchristie Nov 18, 2020
a6cafff
Merge branch 'master' into fm/request-context-manager
tomchristie Nov 18, 2020
262464c
ByteStream -> Iterable[bytes]
tomchristie Nov 18, 2020
9b0cdb0
Drop SyncByteStream, AsyncByteStream in favour of Iterable[bytes]
tomchristie Nov 18, 2020
f9563cf
Neater max_streams_semahore now that we have context-managed flow, ra…
tomchristie Nov 18, 2020
4bacd4a
Merge branch 'master' into fm/request-context-manager
florimondmanca Nov 30, 2020
d7e9b44
Update new tests from master
florimondmanca Nov 30, 2020
824d4ca
Coverage
florimondmanca Nov 30, 2020
807e20b
Merge branch 'master' into fm/request-context-manager
florimondmanca Nov 30, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 6 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,13 @@ Here's an example of making an HTTP GET request using `httpcore`...

```python
with httpcore.SyncConnectionPool() as http:
status_code, headers, stream, ext = http.request(
with http.request(
method=b'GET',
url=(b'https', b'example.org', 443, b'/'),
headers=[(b'host', b'example.org'), (b'user-agent', 'httpcore')]
)

try:
) as response:
status_code, headers, stream, ext = response
body = b''.join([chunk for chunk in stream])
finally:
stream.close()

print(status_code, body)
```
Expand All @@ -61,16 +58,13 @@ Or, using async...

```python
async with httpcore.AsyncConnectionPool() as http:
status_code, headers, stream, ext = await http.arequest(
async with http.arequest(
method=b'GET',
url=(b'https', b'example.org', 443, b'/'),
headers=[(b'host', b'example.org'), (b'user-agent', 'httpcore')]
)

try:
) as response:
status_code, headers, stream, ext = response
body = b''.join([chunk async for chunk in stream])
finally:
await stream.aclose()

print(status_code, body)
```
Expand Down
33 changes: 8 additions & 25 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,55 +2,38 @@

## Async API Overview

The `AsyncHTTPTransport` and `AsyncByteStream` classes provide the base
interface which transport classes need to implement.
The `AsyncHTTPTransport` class provides the base interface which transport classes need to implement.

::: httpcore.AsyncHTTPTransport
:docstring:
:members: arequest aclose

::: httpcore.AsyncByteStream
:docstring:
:members: __aiter__ aclose

The `AsyncConnectionPool` class is a concrete implementation of `AsyncHTTPTransport`.

::: httpcore.AsyncConnectionPool
:docstring:


The `PlainByteStream` and `AsyncIteratorByteStream` classes are concrete implementations of `AsyncByteStream`.

::: httpcore.PlainByteStream
:docstring:

::: httpcore.AsyncIteratorByteStream
:docstring:

---

## Sync API Overview

The `SyncHTTPTransport` and `SyncByteStream` classes provide the base
interface which transport classes need to implement.
The `SyncHTTPTransport` class provides the base interface which transport classes need to implement.

::: httpcore.SyncHTTPTransport
:docstring:
:members: request close

::: httpcore.SyncByteStream
:docstring:
:members: __iter__ close

The `SyncConnectionPool` class is a concrete implementation of `SyncHTTPTransport`.

::: httpcore.SyncConnectionPool
:docstring:

The `PlainByteStream` and `IteratorByteStream` classes are concrete implementations of `SyncByteStream`.
---

## Utilities

::: httpcore.PlainByteStream
:docstring:
The `PlainByteStream` can be used to return a bytestring with both bytes iterable
and async bytes iterable iterfaces.

::: httpcore.IteratorByteStream
::: httpcore.PlainByteStream
:docstring:
18 changes: 6 additions & 12 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,13 @@ Here's an example of making an HTTP GET request using `httpcore`...

```python
with httpcore.SyncConnectionPool() as http:
status_code, headers, stream, ext = http.request(
with http.request(
method=b'GET',
url=(b'https', b'example.org', 443, b'/'),
headers=[(b'host', b'example.org'), (b'user-agent', 'httpcore')]
)

try:
) as response:
status_code, headers, stream, ext = response
body = b''.join([chunk for chunk in stream])
finally:
stream.close()

print(status_code, body)
```
Expand All @@ -61,16 +58,13 @@ Or, using async...

```python
async with httpcore.AsyncConnectionPool() as http:
status_code, headers, stream, ext = await http.arequest(
async with http.arequest(
method=b'GET',
url=(b'https', b'example.org', 443, b'/'),
headers=[(b'host', b'example.org'), (b'user-agent', 'httpcore')]
)

try:
) as response:
status_code, headers, stream, ext = response
body = b''.join([chunk async for chunk in stream])
finally:
await stream.aclose()

print(status_code, body)
```
Expand Down
10 changes: 3 additions & 7 deletions httpcore/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from ._async.base import AsyncByteStream, AsyncHTTPTransport
from ._async.base import AsyncHTTPTransport
Copy link
Member Author

@florimondmanca florimondmanca Nov 30, 2020

Choose a reason for hiding this comment

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

I guess we just need to be careful that this drops SyncByteStream and AsyncByteStream.

We rely on these in a few places in HTTPX: https://github.com/encode/httpx/pull/1399/files#diff-f7cafd3bb44d8d834be724482dacf4ba57c93e7faa62bdce50fc05c00557e222R79

Some other implementations out there would also still be relying on these interfaces.

Should we perhaps keep some aliases in place until 1.0…?

from typing import Iterable, AsyncIterable

SyncByteStream = Iterable[bytes]
AsyncByteStream = AsyncIterable[bytes]

… Or just delay merging these "request context manager" PRs until the next minor release cycle? (We're pinning HTTPCore to a minor in HTTPX anyway.) Yeah, thinking about it — I think that's probably the plan? :-)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I don't think we're sufficiently confident yet about exactly where we're going with this interface,
so I reckon rolling the other stuff (where we are confident) and keeping this hanging a little longer.

from ._async.connection_pool import AsyncConnectionPool
from ._async.http_proxy import AsyncHTTPProxy
from ._bytestreams import AsyncIteratorByteStream, IteratorByteStream, PlainByteStream
from ._bytestreams import PlainByteStream
from ._exceptions import (
CloseError,
ConnectError,
Expand All @@ -19,20 +19,17 @@
WriteError,
WriteTimeout,
)
from ._sync.base import SyncByteStream, SyncHTTPTransport
from ._sync.base import SyncHTTPTransport
from ._sync.connection_pool import SyncConnectionPool
from ._sync.http_proxy import SyncHTTPProxy

__all__ = [
"AsyncByteStream",
"AsyncConnectionPool",
"AsyncHTTPProxy",
"AsyncHTTPTransport",
"AsyncIteratorByteStream",
"CloseError",
"ConnectError",
"ConnectTimeout",
"IteratorByteStream",
"LocalProtocolError",
"NetworkError",
"PlainByteStream",
Expand All @@ -42,7 +39,6 @@
"ReadError",
"ReadTimeout",
"RemoteProtocolError",
"SyncByteStream",
"SyncConnectionPool",
"SyncHTTPProxy",
"SyncHTTPTransport",
Expand Down
37 changes: 8 additions & 29 deletions httpcore/_async/base.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import enum
from types import TracebackType
from typing import AsyncIterator, Tuple, Type
from typing import AsyncContextManager, AsyncIterable, Tuple, Type

from .._types import URL, Headers, T

Expand Down Expand Up @@ -32,43 +32,22 @@ class ConnectionState(enum.IntEnum):
CLOSED = 5 # Connection closed.


class AsyncByteStream:
"""
The base interface for request and response bodies.

Concrete implementations should subclass this class, and implement
the `\\__aiter__` method, and optionally the `aclose` method.
"""

async def __aiter__(self) -> AsyncIterator[bytes]:
"""
Yield bytes representing the request or response body.
"""
yield b"" # pragma: nocover

async def aclose(self) -> None:
"""
Must be called by the client to indicate that the stream has been closed.
"""
pass # pragma: nocover


class AsyncHTTPTransport:
"""
The base interface for sending HTTP requests.

Concete implementations should subclass this class, and implement
Concrete implementations should subclass this class, and implement
the `request` method, and optionally the `close` method.
"""

async def arequest(
def arequest(
self,
method: bytes,
url: URL,
headers: Headers = None,
stream: AsyncByteStream = None,
stream: AsyncIterable[bytes] = None,
ext: dict = None,
) -> Tuple[int, Headers, AsyncByteStream, dict]:
) -> AsyncContextManager[Tuple[int, Headers, AsyncIterable[bytes], dict]]:
"""
The interface for sending a single HTTP request, and returning a response.

Expand All @@ -79,17 +58,17 @@ async def arequest(
of (scheme, host, port, path).
* **headers** - `Optional[List[Tuple[bytes, bytes]]]` - Any HTTP headers
to send with the request.
* **stream** - `Optional[AsyncByteStream]` - The body of the HTTP request.
* **stream** - `Optional[AsyncIterable[bytes]]` - The body of the HTTP request.
* **ext** - `Optional[dict]` - A dictionary of optional extensions.

** Returns:**

A four-tuple of:
A context manager yielding a four-tuple of:

* **status_code** - `int` - The HTTP status code, such as `200`.
* **headers** - `List[Tuple[bytes, bytes]]` - Any HTTP headers included
on the response.
* **stream** - `AsyncByteStream` - The body of the HTTP response.
* **stream** - `AsyncIterable[bytes]` - The body of the HTTP response.
* **ext** - `dict` - A dictionary of optional extensions.
"""
raise NotImplementedError() # pragma: nocover
Expand Down
20 changes: 10 additions & 10 deletions httpcore/_async/connection.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
from ssl import SSLContext
from typing import Optional, Tuple, cast
from typing import AsyncIterable, AsyncIterator, Optional, Tuple, cast

from .._backends.auto import AsyncBackend, AsyncLock, AsyncSocketStream, AutoBackend
from .._compat import asynccontextmanager
from .._exceptions import ConnectError, ConnectTimeout
from .._types import URL, Headers, Origin, TimeoutDict
from .._utils import exponential_backoff, get_logger, url_to_origin
from .base import (
AsyncByteStream,
AsyncHTTPTransport,
ConnectionState,
NewConnectionRequired,
)
from .base import AsyncHTTPTransport, ConnectionState, NewConnectionRequired
from .http import AsyncBaseHTTPConnection
from .http11 import AsyncHTTP11Connection

Expand Down Expand Up @@ -72,14 +68,15 @@ def request_lock(self) -> AsyncLock:
self._request_lock = self.backend.create_lock()
return self._request_lock

@asynccontextmanager
async def arequest(
self,
method: bytes,
url: URL,
headers: Headers = None,
stream: AsyncByteStream = None,
stream: AsyncIterable[bytes] = None,
ext: dict = None,
) -> Tuple[int, Headers, AsyncByteStream, dict]:
) -> AsyncIterator[Tuple[int, Headers, AsyncIterable[bytes], dict]]:
assert url_to_origin(url) == self.origin
ext = {} if ext is None else ext
timeout = cast(TimeoutDict, ext.get("timeout", {}))
Expand All @@ -103,7 +100,10 @@ async def arequest(
logger.trace(
"connection.arequest method=%r url=%r headers=%r", method, url, headers
)
return await self.connection.arequest(method, url, headers, stream, ext)
async with self.connection.arequest(
method, url, headers, stream, ext
) as response:
yield response

async def _open_socket(self, timeout: TimeoutDict = None) -> AsyncSocketStream:
scheme, hostname, port = self.origin
Expand Down
Loading