[RFC] Discussion PR for HTTP/2 support (DO NOT MERGE). #882
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What these changes does?
This is an example of the most basic HTTP/2 support in aiohttp. This is nowhere near close to production ready: instead, it's intended to spur a longer discussion about how best to add HTTP/2 support to aiohttp. This code does the following:
This code does not support any of the following:
I opened this because I believe that if aiohttp wants to support HTTP/2 it requires a very substantial change, and I did not want to work on that in isolation without discussing it with the aiohttp community. However, it's always tough to discuss such a change without having some concrete code to look at, so this provides that code.
How to test your changes?
The following test file is a variation on the example from the documentation, and can be tested using the Python hyper library.
This is the server file:
To test it, install the Python hyper library (
pip install hyper
) and then runhyper --h2 GET http://localhost:8080/name
.Related issue number
#863, #320
Discussion
When trying to implement HTTP/2 support in aiohttp, I discovered that aiohttp's architecture and API make it remarkably tricky to plumb HTTP/2 support through in a coherent and sensible way. The biggest problems here are:
ServerHttpProtocol
and*Parser
classes. For example,web_urldispatcher
contains a_defaultExpectHandler
that writes directly to the transport rather than passing that write through theServerHttpProtocol
or equivalent object. More generally,HttpMessage
and friends all write directly to the transport, which means that to make HTTP/2 work we need to propagate quite a lot of state through aiohttp to get it from theServerHttp2Protocol
to those objects.ServerHttpProtocol
, but because there are so many different objects that have access to the backing transport (and also the reader/writer objects) it is very difficult to enforce that constraint. Additionally, for HTTP/2 they'd all need a correlator (their stream ID), which makes a further substantial change to the codebase. That wouldn't be too bad, except...ServerHttpProtocol
and friends all expect to be subclassed. Because of this, there is likely quite a lot of third-party code that subclasses aiohttp's classes and reaches inside of them. That makes it very difficult to do a substantial refactor to the code. If aiohttp was unwilling to break that code, they'd need either to do a big breaking semver-major change (which will annoy a lot of people) or write an entirely parallel stack that is capable of doing HTTP/1.1 and HTTP/2 transparently and then ask people to migrate to that. Either seems like quite a lot of work.Basically, from where I'm sat I think that adding HTTP/2 support transparently into aiohttp requires a very substantial change to the library, and I'm unwilling to do that without discussing with the community and maintainers first. Is there any interest in having such a substantial change made to aiohttp, or do the maintainers and community believe that HTTP/2 support is not worth it?