feat: axum v0.8 support - #168
Conversation
JakkuSakura
left a comment
JakkuSakura
left a comment
There was a problem hiding this comment.
Thank you for the quick update to axum v0.8. I see there are a few changes regarding Sync. Any reason for these? I'm afraid that they may break old code. The rest looks fine
| fn test_nesting_with_nondefault_state() { | ||
| let _app: ApiRouter = ApiRouter::new() | ||
| .nest_api_service("/", ApiRouter::new().with_state(1_isize)) | ||
| .nest_api_service("/home", ApiRouter::new().with_state(1_isize)) |
There was a problem hiding this comment.
why is the change here needed?
| pub fn nest_service<T>(mut self, path: &str, svc: T) -> Self | ||
| where | ||
| T: Service<Request<Body>, Error = Infallible> + Clone + Send + 'static, | ||
| T: Service<Request<Body>, Error = Infallible> + Clone + Send + 'static + std::marker::Sync, |
There was a problem hiding this comment.
why add new Sync trait bound?
There was a problem hiding this comment.
Tbh. It's what the compiler complained about
|
It will break the old code. That's for sure. The reason for this is, that Axum dropped the support for the async—trait. |
|
I dug into the axum code. It's not because they dropped async-trait, but they added Sync when they dropped |
|
I made it into a Draft to test it on my own Project. I just saw, that there are not that many tests |
|
Could you bump aide to 0.14.0 as it's a breaking change unavoidably? Other crates under crates/* and examples/* also need to bump version and use latest aide + axum combo |
3abeb84 to
aefb133
Compare
|
To make sure there isn't a misunderstanding. I cant fix the aide-axum-typed-multipart crate, because it hast implemented the change itself. In my Project everything worked fine. But im not a fan of the nest_api_service changes :/ |
|
I see. This PR is needed |
|
I don't think, we should wait for it. I tried implementing it right now, but this package need to update the axum-test-helper package to get the tests to run. |
|
Question: |
aefb133 to
90cadd1
Compare
|
The Axum-typed-multipart works now. The Action Fails, due to the missing crates.io entry. |
feat: axum-typed-multipart
90cadd1 to
6cd56a2
Compare
|
I suggest just wait for a while. |
|
It was a bug. I missed something. I forgot to update the aide version in one of the crates. Thats why it failed to build them from the path. Now with the last "force-push" it should work |
|
Dang, I missed this PR! Just posted my own (#170). Let me see where the differences are.. |
|
Your GithubAction is failing. Thats one of the differences xD |
|
Yeah, I failed to update a few things in packages we likely want to drop anyways. Have opened a separate PR for that. Still, we can probably merge this first and get all of the other improvements from my PR in separately. I'll review your changes + the existing comments. |
|
@JakkuSakura hope you don't mind that I merged this. There's some things I still want to improve about the upgrade, but it seemed like a good first step to get it in. |
|
#170 has now been rebased on top of this. |
|
@jplatte it's ok. I'm just a bit conservative on this kind of PR. It has larger impact to users. With your approval it would be sufficient |
4 participants
fixes #167
Based on this announcement