Repository navigation
Add OpenConnection base class - #616
Conversation
| ) -> None: | ||
| if http_version == "HTTP/2": | ||
| self.h2_connection = HTTP2Connection( | ||
| self.open_connection = HTTP2Connection( |
There was a problem hiding this comment.
This is the only place where we reference HTTP2Connection in this file, so we'll be able to lazily-import it here, and fail if h2 isn't installed. 馃憤
| else: | ||
| assert http_version == "HTTP/1.1" | ||
| self.h11_connection = HTTP11Connection(stream, on_release=on_release) | ||
| self.set_open_connection(http_version, socket=stream, on_release=on_release) |
There was a problem hiding this comment.
Note that we've got an outstanding stream vs socket instance here (following up #601). Wondering if should push the renaming further with .open_tcp_socket() and .open_uds_socket() on the ConcurrencyBackend interface?
lovelydinosaur
left a comment
lovelydinosaur
left a comment
There was a problem hiding this comment.
Seems good, yup!
I think there may be a bit more we can do to avoid reaching into h11 internals from here, but we can address that seperately as needed.
| else: | ||
| assert http_version == "HTTP/1.1" | ||
| self.h11_connection = HTTP11Connection(stream, on_release=on_release) | ||
| self.set_open_connection(http_version, socket=stream, on_release=on_release) |
|
Thanks @tomchristie - I see what you mean about the remaining h11 internals being accessed in HTTPConnection. Seems separate from the scope of this PR so I鈥檒l merge and we can address it later! |
To progress towards resolving #496, we need to isolate the manipulation of
HTTP2ConnectioninHTTPConnection.This PR (inspired by #604) introduces a base
OpenConnectioninterface thatHTTP11ConnectionandHTTP2Connectionimplement, which allowsHTTPConnectionto hold a singleopen_connectionobject instead of one ofh11_connectionorh2_connection.