Skip to content
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

Upgrade to axum ^0.7 #227

Open
lebenitza opened this issue Apr 22, 2024 · 18 comments
Open

Upgrade to axum ^0.7 #227

lebenitza opened this issue Apr 22, 2024 · 18 comments

Comments

@lebenitza
Copy link

Are there any plans to migrate to axum v0.7.x ?

https://github.com/tokio-rs/axum/releases/tag/axum-v0.7.0

This comes with other dependencies that need to be upgraded too, mentioned in the 0.7.0 release. I tried figuring this out but I am stuck at

error[E0119]: conflicting implementations of trait `FromRequest<_, axum_core::extract::private::ViaParts>` for type `event::Event`
  --> src/binding/axum/extract.rs:16:1
   |
16 | / impl<S, B> FromRequest<S, B> for Event
17 | | where
18 | |     B: Body + Send + 'static,
19 | |     B::Data: Send,
20 | |     B::Error: Into<BoxError>,
21 | |     S: Send + Sync,
   | |___________________^
   |
   = note: conflicting implementation in crate `axum_core`:
           - impl<S, T> FromRequest<S, axum_core::extract::private::ViaParts> for T
             where S: Send, S: Sync, T: FromRequestParts<S>;
   = note: downstream crates may implement trait `axum::extract::FromRequestParts<_>` for type `event::Event`
   = note: upstream crates may add a new impl of trait `http_body::Body` for type `axum_core::extract::private::ViaParts` in future versions

For more information about this error, try `rustc --explain E0119`.
error: could not compile `cloudevents-sdk` (lib) due to 1 previous error
@Lazzaretti
Copy link
Member

Hi @lebenitza ,
Sorry for the delay. I think it makes sense to update it. Would you like to contribute a PR? I'm happy to support you if you need help.

@ozabalaferrera
Copy link
Contributor

I taking a quick look at what it would take to do this, and it seems that upgrading to http ^1.0 is unavoidable.
For example, I can't find a good way to create an http 1.1.0 response from an http 0.2.12 response to implement IntoResponse for Event.

Is that what you would expect, @Lazzaretti ?

@Lazzaretti
Copy link
Member

I think you are right, the http crate has to be updated. If this is the case, I'm fine with an upgrade.

@ozabalaferrera
Copy link
Contributor

Sounds good. I have some travel scheduled, but I can take this up in a few weeks if no one else has by then.

@devsprint
Copy link

@Lazzaretti when do you plan a new release ?

@ozabalaferrera
Copy link
Contributor

I've been working on this on and off, but got stuck on implementing the axum(-core) FromRequest trait for Event. It complains (see below) about a conflict resulting from a generic implementation of FromRequest for types that implement FromRequestParts, but that FromRequestParts is not implemented for Event. I'll keep trying to figure it out, but any suggestions would be appreciated.

error[E0119]: conflicting implementations of trait `FromRequest<_, axum_core::extract::private::ViaParts>` for type `event::Event`
  --> src\binding\axum\extract.rs:17:1
   |
17 | / impl<S, B> FromRequest<S, B> for Event
18 | | where
19 | |     B: Body + Send + 'static,
20 | |     B::Data: Send,
21 | |     B::Error: Into<BoxError>,
22 | |     S: Send + Sync,
   | |___________________^
   |
   = note: conflicting implementation in crate `axum_core`:
           - impl<S, T> FromRequest<S, axum_core::extract::private::ViaParts> for T
             where S: Send, S: Sync, T: FromRequestParts<S>;
   = note: downstream crates may implement trait `axum::extract::FromRequestParts<_>` for type `event::Event`
   = note: upstream crates may add a new impl of trait `http_body::Body` for type `axum_core::extract::private::ViaParts` in future versions

@nicmr
Copy link

nicmr commented Jul 10, 2024

@ozabalaferrera Could this be the same issue as the one described in this SO response? https://stackoverflow.com/a/77816211
i.e. FromRequest signature has changed from FromRequest<S, B> to FromRequest<S>

Linking the references shared in the response directly:

@ozabalaferrera
Copy link
Contributor

@nicmr, that was it, thanks!

@ozabalaferrera
Copy link
Contributor

@Lazzaretti, I've not evaluated what is needed to update web (server) frameworks other than axum. The changes I've prepared use feature gates to control dependency versions and function signatures, such that only the axum feature is affected/updated. However, that breaks some feature combinations (see below). As a result, the test jobs would have to change to remove the use of --all-features. Are you satisfied with this approach, or should I hold off and try for a larger PR that updates as many of the frameworks as possible?

axum and (nats or rdkafka) are okay, these are not:

#[cfg(all(
    feature = "axum",
    any(
        feature = "http-binding",
        feature = "actix",
        feature = "reqwest",
        feature = "warp",
        feature = "poem"
    )
))]
compile_error!("feature `axum` cannot be used with features `http-binding`, `actix`, `reqwest`, `warp`, or `poem`");

I figured I'd ask before putting up the PR, but I'm happy to put up a draft if you'd like to take a look.

@Lazzaretti
Copy link
Member

@Lazzaretti when do you plan a new release ?

I don't have a strict timeline. But I think we soon need a new release :)

@Lazzaretti
Copy link
Member

@Lazzaretti, I've not evaluated what is needed to update web (server) frameworks other than axum. The changes I've prepared use feature gates to control dependency versions and function signatures, such that only the axum feature is affected/updated. However, that breaks some feature combinations (see below). As a result, the test jobs would have to change to remove the use of --all-features. Are you satisfied with this approach, or should I hold off and try for a larger PR that updates as many of the frameworks as possible?

axum and (nats or rdkafka) are okay, these are not:

#[cfg(all(
    feature = "axum",
    any(
        feature = "http-binding",
        feature = "actix",
        feature = "reqwest",
        feature = "warp",
        feature = "poem"
    )
))]
compile_error!("feature `axum` cannot be used with features `http-binding`, `actix`, `reqwest`, `warp`, or `poem`");

I figured I'd ask before putting up the PR, but I'm happy to put up a draft if you'd like to take a look.

@jcrossley3 any suggestion from your side?

@ozabalaferrera If you want to ping me on Slack and we can discuss this a bit in detail: https://cloud-native.slack.com/team/UTV7ZGPAR

@jcrossley3
Copy link
Contributor

@jcrossley3 any suggestion from your side?

My inclination is to keep --all-features working, even if that requires updating the other frameworks to later (stable) versions. But I have no suggestions regarding the best way of doing that.

@ozabalaferrera
Copy link
Contributor

@jcrossley3, understood. I already started working on another branch with broad dependency updates. warp might be troublesome because it is not on hyper and http versions 1. We'll see how it goes 😄

@Lazzaretti, I'll reach out on slack if I have any other questions.

@ozabalaferrera
Copy link
Contributor

ozabalaferrera commented Dec 4, 2024

This should resolved by PR #233 and good to go in version 0.8.0 when it's released.

@FalkWoldmann
Copy link

Hi @ozabalaferrera, sorry to bug you, but we use the cloudevents-sdk and are currently stuck on an old rdkafka version because of it. Do you know when you are able to release the new version?

@ozabalaferrera
Copy link
Contributor

@FalkWoldmann, I'll take a look at the rdkafka dependency and see if I am able to upgrade it. To be clear, I am not a maintainer and I cannot "release" a new version. If I figure it out I will open a PR though.

@fourlexboehm
Copy link

What are the blockers on a new release? I've been tracking main with axum 0.7/.8 and it's working well. I'm happy to help out if there's something I can do to get this released.

@jcrossley3
Copy link
Contributor

What are the blockers on a new release? I've been tracking main with axum 0.7/.8 and it's working well. I'm happy to help out if there's something I can do to get this released.

I'm just back from PTO, sorry for the late reply. I'm open to getting a release out this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants