-
Notifications
You must be signed in to change notification settings - Fork 1k
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
chore: Update from tower 0.4 to 0.5 #2004
Conversation
This was done already in #1892. Unfortunately it is a change in the public API so we'd need everyone to migrate. Not sure if this change is worth doing an incompatible bump on its own. |
Most of the ecosystem including axum has already migrated to tower 0.5 so I'm not sure how it's a choice of whether to do it or not. Not doing this locks tonic users out of using any tower 0.5-based middleware and any progress done in tower-http 0.6+, as well as contributing to the doubled dependency problem that seems so prevalent in the Rust ecosystem. So in my opinion, this is most certainly worth doing a 0.13.0 release for. #1892 is equivalent to this PR except for the fact that this PR applies better formatting to some Cargo.toml-s in the tonic workspace as an off-topic benefit. |
Actually, let me do this in this same PR, would be neat. |
There's already a PR for that one, too: #1556. For now, I don't think we can enable it by default for now given our MSRV, and I think the stance of the Rust Project is that RPITIT is "discouraged for general use in public traits" due to semver hazards while we don't have RTN. (And I definitely don't think we should have these in the same PR.) |
Looking at the way this was formulated in https://blog.rust-lang.org/2023/12/21/async-fn-rpit-in-traits.html, the actual concern that is to be considered with this are possible changes in trait bounds. But that argument is not really applicable to tonic, since async_trait already enforces As for the MSRV argument, 1.82 is gonna be out soon so 1.75 was already a while ago, and current MSRV is 1.71, so is it really that significant of a bump if we're already talking about making breaking changes? |
Anyway, if the async_trait removal is not justifiable, I still think either this needs to be shipped or if we're really committing for tonic holding back its tower version, hold back the axum version with a |
This pull request is a duplicate of an existing pull request. And since "async fn in traits" is not related to updating of tower dependency, it doesn't seem like an appropriate place to discuss it. |
Sure, but I would like for at least the original to get merged so that tonic isn't seemingly purposefully lagging behind tower, while axum which it uses as an internal framework already has upgraded. |
Motivation
#2003
Solution
Bumped dependencies