-
-
Notifications
You must be signed in to change notification settings - Fork 792
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
HTTP2 support #14
Comments
@leonardoo that's a client library, though - this would need a server library. This may be a case where I need to write something myself that works with Twisted/asyncio. |
hyper also has the necessary pieces to build a server, but under a different repo. They have an example of how it can be done. |
@andrewgodwin Here's a question. I can get a formal HTTP/2 server into Twisted (see Twisted issue 7460), but that'll probably take a while. However, it might be nice to see if we can get something off the ground faster. Would it be valuable for me to try to spike out an example implementation for channels directly, depending on Twisted and hyper-h2, and let you see how that looks? |
I'm actually working on redoing the Twisted WebSocket interface server into a combined HTTP and WebSocket one, despatching based on upgrade header, so it might be nice if a spike kind of like that happened? Not quite sure yet. |
When you say "a spike kind of like that", do you mean a spike into your refactored WebSocket server, or a spike that refactored the WebSocket server into a combined HTTP/2+WS, or a combined HTTP+HTTP/2? |
Well my ideal would be a combined HTTP1/2/WebSocket server, with dynamic negotiation (switch based on upgrade header or the HTTP2 preamble bytestring) - right now I have to rewrite the current WebSocket server to have the HTTP parser as the "first" one, and then override the request handling to drop it over to the WebSocket protocol class if it sees the Upgrade header. |
@andrewgodwin You shouldn't really have to switch based on the HTTP/2 preamble. For cleartext h2 you should get an Upgrade: header, for TLS-based you should get NPN/ALPN. Regardless, this should be plenty do-able. Do you want me to hold off until you have that in a sensible shape and then have me jump in and see if I can fit H2 in there too? |
@Lukasa That might be best, yeah - I have some code half-done for the mixed server, but I'm still straightening out Twisted Web to insert hooks in the right place. I'll ping you when I land it? |
Sounds good to me. =) |
nghttp2 has python bindings: https://nghttp2.org/documentation/package_README.html#python-bindings |
@fdemmer nghttp2's bindings should be unnecessary. The Twisted patches for HTTP/2 are being code reviewed as we speak, and when they land the reference ASGI server should gain HTTP/2 support automatically. |
However, if @andrewgodwin wants to see something earlier, we can probably also pull in some of the hyper-h2 stuff and have daphne land its HTTP/2 support before Twisted does. Up to him really. |
I'd rather wait for the Twisted code to land; it's not a huge priority to get it working, it would just be nice. |
Closing in favour of django/daphne#30. |
Likely via a new interface server - perhaps one that can also support WebSockets and HTTP1 all in one big go. Needs some research into the current state of python HTTP2 support.
The text was updated successfully, but these errors were encountered: