update hyper and axum #137
Conversation
| while let Some(conn) = listener.try_next().await? { | ||
| let service = service.clone(); | ||
| let dropref = dropref.clone(); | ||
| // Spawn a task to handle the connection. That way we can multiple connections |
There was a problem hiding this comment.
nit: maybe "That way we can handle multiple"
| let mut make_service = app.into_make_service_with_connect_info::<SocketAddr>(); | ||
|
|
||
| let server = async move { | ||
| while let Some(conn) = listener.try_next().await? { |
There was a problem hiding this comment.
This block shows up a bit, could it be made into a re-usable component for ourselves and users, or does that get too messy?
There was a problem hiding this comment.
Yeah, so this is what I originally tried to do and burned most of monday on.
We could
- Support
hyper'sServicetrait directly, and expose a generic method that takes one of those. Unfortunately, this means that we can't useaxum'sConnectedtrait. - Support
tower'sServicetrait. This lets us supportaxumand itsConnectedtrait better, but ties us to thetowerecosystem.hyperseems like a better bet imo than the higher-leveltowerAPIs, at least for now.
The core issue with hyper's Service is that it expects a Request, which is already interpreted at the protocol level and doesn't include things like the remote addr. Meanwhile, axum's Connected expects that context to be plumbed in through the tower::Service side-channel by seeding it with the original connection stream first before handing the remote-addr-seeded service off to the hyper API.
It's all a mess and I think I'd prefer to support the lowest-common-denominator for now, which seems to be hyper. I'm frustrated that they removed the Accept trait and Server struct and rebased their Service to operate on Requests instead of the connection without providing an alternative. Ideally, axum::serve would've filled that gap, but it's explicitly dependent on TcpListener, so it's a no-go.
There was a problem hiding this comment.
Eek, makes sense to just be raw hyper and everything else DIY, it's sad they dropped that. I do wonder if these accept loops could be moved to a helper in our sdk so the examples and users would have less to do directly.
Hopefully, as |
jrobsonchase
force-pushed
the
josh/hyper-one-dot-oh
branch
from
f606ae6 to
cc9947f
Compare
|
When is this PR expected to be integrated? |
|
@jrobsonchase @bobzilladev Any chance this will get merge any time soon? I could really use this, since my project is dependent on Hyper 1.0+. Let me know if there is anything I can do to speed this up. |
Significant breakage, since hyper 1.0 introduced significant breakage.
Of note:
Accepttrait, and no more hyper-provided server. Examples are all modeled after the axum serve-with-hyper example.Connnow implements the hyper read/write traits, because why not?All in all, not super happy about this change. Hyper used to provide a nice pluggable interface where you could bring-your-own listener type, and that's been entirely done away with. Axum didn't fill in the gap very well, and their
servefunction explicitly takes a TCP listener. Not sure we want to provide yet another accept-serve loop.Since hyper is 1.0 and stable though, I feel better about providing implementations of the traits that they do offer. Maybe the rest of the ecosystem will catch up and figure out how to not depend on
TcpListenerdirectly.Resolves #126
Resolves #136