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

Bump rust lib to Hyper 1.0 #1146

Closed
aldofunes opened this issue Dec 14, 2023 · 9 comments · Fixed by #1197
Closed

Bump rust lib to Hyper 1.0 #1146

aldofunes opened this issue Dec 14, 2023 · 9 comments · Fixed by #1197
Assignees
Labels
lib/rust Rust client library

Comments

@aldofunes
Copy link

aldofunes commented Dec 14, 2023

It's actually about the libs

Old report

Feature Request

Upgrade axum to 0.7

Motivation

I have an axum server that uses svix, both as an emitter and a consumer. With the recent changed to 1.0 of the hyper and http crates, a bunch of breaking changes were introduced in the ecosystem. Axum's 0.7 version now supports these 1.0 releases. However, since it uses a different http create version, the validation of webhooks fails due to the HeaderMap's version difference in the function signature.

I have put a lot of effort into migrating our codebase to axum 0.7 and this is the last piece of the puzzle.

Proposal

Having skimmed through the lib's code, this should not introduce breaking changes, but will probably require custom feature flags like axum06 and axum07.

Since I'm blocked, and you guys are probably busy, I can do these changes and submit a PR; but I might need some guidance into any standards or ways of doing things you guys have.

Alternatives

Te alternative is to go back to 0.6, which will work for the time being, but I'm assuming this will eventually be needed

@aldofunes aldofunes changed the title Bump axum to 0.7 Bump axum to 0.7 in rust crate Dec 14, 2023
@tasn tasn added the lib/rust Rust client library label Dec 15, 2023
@tasn
Copy link
Member

tasn commented Dec 15, 2023

It sounds like you're referring to the Rust lib, not the server itself, right?

@aldofunes
Copy link
Author

Yes, the rust lib.

Having just gone through the upgrade on a server with a pretty similar setup (axum+aide), I know the server will be a huge undertaking.

I understand if you guys are planning to upgrade the whole think in one go

@tasn
Copy link
Member

tasn commented Dec 15, 2023

Good to know it's a huge undertaking. We were going to do it but the changelog looked significant so we decided to defer it for a bit. :)

We can probably upgrade the libs for now anyway and worry about the server at a later stage.

P.S, what was the most annoying part of the upgrade?

@tasn
Copy link
Member

tasn commented Dec 15, 2023

Quick glance: one of our deps (Reqwest) is incompatible with Hyper 1.0 still.
We can take a look whether using the OpenAPI generator with Hyper works when forcing it to use Hyper 1.0.

@tasn tasn changed the title Bump axum to 0.7 in rust crate Bump rust lib to Hyper 1.0 Dec 18, 2023
@tasn
Copy link
Member

tasn commented Dec 18, 2023

Hey @aldofunes, you mentioned you'd be open to opening a PR, is it still the case? Just need to update the openapi spec generator to generate Hyper instead of reqwest, and then up the dep requirements of Hyper to >=1 and hope it works. :P

@Srlion
Copy link

Srlion commented Jan 8, 2024

Any updates on this? at least update the lib to be able to use webhook.verify

@tasn
Copy link
Member

tasn commented Jan 8, 2024

If we don't hear back from @aldofunes about this, we'll take a jab at it next week. Thanks for bumping the issue!

@Srlion
Copy link

Srlion commented Jan 28, 2024

@tasn Any news?

@tasn
Copy link
Member

tasn commented Jan 28, 2024

We started looking at it last week. It looks like some of our dependencies, namely the AWS Rust SDK depends on Hyper 0.10 (tracking ticket: awslabs/aws-sdk-rust#977).

Though we'll figure out a way to support both. We have a new team member joining the team on Thursday and I plan for him to fix it.

svix-gabriel added a commit that referenced this issue Feb 12, 2024
~~Depends on #1193.~~
Closes #1146.

Probably best to review commit by commit, though most of the changes are
in a single commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib/rust Rust client library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants