Upgrade to Hyper 1.0 & Axum 0.7 #1670
Conversation
Some low-hanging import fixes
This commit is primarily converting Request and Response types within the transport module to Axum 0.7 Request/Response. There is still more to come to finish this conversion. There are also small changes such as updating the hyper service builder syntax. Over the course of this commit,it was discovered that hyper-util is missing `http2_max_pending_accept_reset_streams`.
Replaced with `tokio::net::TcpStream`. Inspired by hyperium/hyper#2850
- use `Frame`. - update `poll_data` to `poll_frame` - remove `poll_trailers` in most places I am not sure that a simple rename of `poll_data` to `poll_frame` is right. I also left one instance of `poll_trailers` in *tonic/src/codec/decode.rs*.
`is_connect` was deprecated when the higher-level client was removed from hyper. The corresponding comments are removed as well.
`http::Extensions::insert` requires `T: Clone` so we add it.
Provides adaptors between the Hyper-1.0 stable traits and traits needed by tonic and axum to send and recieve requests as a client.
|
I'd like to propose that this be reviewed as-is, and that we add follow-up issues for the remaining todos, which will all require PRs to land and then releases to happen for |
|
works perfectly after |
remove `--ignore` option when pr(when hyperium/tonic#1670) is merged. Signed-off-by: Phoeniix Zhao <Phoenix500526@163.com>
remove `--ignore` option when pr(when hyperium/tonic#1670) is merged. Signed-off-by: Phoeniix Zhao <Phoenix500526@163.com>
examples/Cargo.toml
Outdated
| rustls-pemfile = { version = "1", optional = true } | ||
| tower-http = { version = "0.4", optional = true } | ||
| tokio-rustls = { version = "0.26.0", optional = true } | ||
| hyper-rustls = { version = "0.27.0", features = ["http2"], optional = true } |
There was a problem hiding this comment.
It would be nice if there were feature flags to switch between the crypto backends.
Defaulting to aws-lc-rs pulls in a cmake, nasm requirement, impacts platform support, etc.
Trading that off for FIPS compliance isn't really needed in most cases.
There was a problem hiding this comment.
Which crypto provider tonic should default to is up to the maintainers, I guess.
But reqwest seems to be going the route of sticking to ring as the default crypto backend.
There was a problem hiding this comment.
I think that is a good idea, but I don't think it should be included in this PR - this PR is already complicated as is. We can create an issue and work on multi-backend support separately once this lands.
There was a problem hiding this comment.
Yeah, definitely. I just felt like the comment fit best here for the time being since the PR isn't merged yet and I probably would have forgotten about it if I hadn't written it down
See rust-lang/cargo#10788 for the full issue. When enabling a feature of a dependency, the syntax `tokio/rt` implicitly required a feature `tokio` to exist in 1.70 of cargo. By 1.77 this has been fixed. To work around this, we can use "weak feature dependencies" like `tokio?/rt` which enables the `rt` feature of tokio only if tokio is itself an enabled dependency. Also adding `dep:tokio` to the list of dependencies ensures that this is always true. When the MSRV is upgraded again, we can probably remove the weak `?` and the `dep:tokio` in these cases.
transport uses tokio::select!
aws-lc-crypto doesn't seem to compile by defualt on windows. Ideally, this should be a feature we toggle between.
It is only used by the TLS feature.
NOTE: These changed between prost-build 0.12.3 and 0.12.4, adding the top line ocmment which includes how they were generated.
axum 0.7 requires the `#[diagnostic]` attribute namespace on nightly.
This requires CMake. Once again, a fast-follow should enable aws-lc-rs as a feature and the ability to switch between ring and aws-lc-rs
Found some places where links were not resolving correctly, this corrects them.
According to [hyperium#689](hyperium#689) streams should end after the first error. Therefore, when we find an error in the response step, discard the trailers so that the stream ends.
|
cc @tottoto @LucioFranco can we get this patch a review? This should be one of the last blockers for many downstreams upgrade to hyper 1.0. |
|
@alexrudy FYI hyperium/hyper-util#102 is merged now. |
Current Status: This is ready for CI & review!
This is a continuation of PR #1595 by @ikrivosheev.
TODO:
poll_frame. Still some work to do here about preserving data and trailers ifpoll_framereturns a frame type we aren't expecting.hyper-util)hyper-util, but also not too hard to do independently, followingaxum's example.)max_pending_accept_reset_streams(needs a release with PR #102 inhyper-util)http2_only(needs PR #111 inhyper-util.ConnectErrorsocket2dependency to support TCPSO_NODELAYandSO_KEEPALIVE