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 http 1.0 #237

Closed
jonhoo opened this issue Nov 25, 2023 · 17 comments
Closed

Upgrade to http 1.0 #237

jonhoo opened this issue Nov 25, 2023 · 17 comments

Comments

@jonhoo
Copy link

jonhoo commented Nov 25, 2023

hyper has reached 1.0, and with it the http crate did as well. Since http is part of the public API for oauth2 (e.g., via the pub use http re-export), it'd be nice if oauth2 followed suit!

I took a stab at doing it myself, and it doesn't look too painful, except for the fact that there's not yet a version of reqwest that uses http 1.0 (seanmonstar/reqwest#2039). Which means this is blocked for the time being, but I figured it'd still be useful to have a tracking issue.

@ramosbugs
Copy link
Owner

sounds good! let's do this once reqwest supports 1.0, and that will require a major version bump of this crate and the downstream openidconnect crate.

@MarijnS95
Copy link
Contributor

I took a stab at doing it myself, and it doesn't look too painful,

How much of this have you got done already? Per #236 there's now consent to see if te Http{Request,Response} can be replaced with those from the http crate. Indeed, the 1.0 release seems to mainly solidify the API rather than introduce breaking changes.

(I came around this crate for the improved http<->ureq conversions that sparked #236, and am also interested in bumping to http 1.0.0 to be able to utilize that).

@jonhoo
Copy link
Author

jonhoo commented Nov 26, 2023

I stopped fairly early on when I realized that we'd need a release of reqwest that also uses http 1.0 first. That's not because of the public API, but rather just for type compatibility.

@jonhoo
Copy link
Author

jonhoo commented Nov 26, 2023

Also worth linking this, which it took me a bit to find: https://hyper.rs/guides/1/upgrading/

@MarijnS95
Copy link
Contributor

Does this crate use hyper, or only a few methods in reqwest (which uses hyper under the hood)?

@ttys3
Copy link

ttys3 commented Dec 1, 2023

for reqwest, we need wait upstream seanmonstar/reqwest#2039

@ramosbugs
Copy link
Owner

I've started merging breaking changes for the next major release (5.0) of this crate into main. Hopefully this change will be done soon enough to make it into 5.0 (no concrete timeframe).

@nicolaspernoud
Copy link

Would it be possible to make a beta 5.0 branch depending on

reqwest = { git = "https://github.com/seanmonstar/reqwest.git", branch = "hyper-v1" }

So that the users of this crate work on their migration to be ready for the reqwest release ?

@seanpianka
Copy link

It's been merged! seanmonstar/reqwest#2039

@ramosbugs
Copy link
Owner

It's been merged! seanmonstar/reqwest#2039

I saw! I should have a branch ready soon, but I won't be able to cut a release until reqwest 0.12 is released with the changes

ramosbugs added a commit that referenced this issue Mar 9, 2024
BREAKING CHANGES: This changes several exported types, including
`HttpRequest` (`http::Request`) and `HttpResponse` (`http::Response`),
as well as the `reqwest` client (`reqwest::Client` and
`reqwest::blocking::Client`).

Resolves #237.
ramosbugs added a commit that referenced this issue Mar 9, 2024
BREAKING CHANGES: This changes several exported types, including
`HttpRequest` (`http::Request`) and `HttpResponse` (`http::Response`),
as well as the `reqwest` client (`reqwest::Client` and
`reqwest::blocking::Client`).

Resolves #237.
@ramosbugs
Copy link
Owner

Alright, I've upgraded to http 1.0 in the http-1.0 branch. Until a release is cut, anyone can test it as follows:

oauth2 = { git = "https://github.com/ramosbugs/oauth2-rs.git", rev = "03bc493a3d90643c6deb77e0da12e9a1a90d9be5" }

@LorenzoLeonardo
Copy link
Contributor

@ramosbugs what would be the minimum required Rust compiler version for this?

@LorenzoLeonardo
Copy link
Contributor

Testing done, and this is working. Please merge

@ramosbugs
Copy link
Owner

@ramosbugs what would be the minimum required Rust compiler version for this?

should still be 1.65

Testing done, and this is working. Please merge

still waiting on a reqwest 0.12 release: https://github.com/seanmonstar/reqwest/releases

@ramosbugs
Copy link
Owner

ramosbugs commented Mar 21, 2024

5.0.0-alpha.3 is now released with dependencies on http 1.0 and reqwest 0.12

@jonhoo
Copy link
Author

jonhoo commented Mar 30, 2024

I assume you mean 5.0.0-alpha.3? 😅

@ramosbugs
Copy link
Owner

fixed, thanks! 🤦‍♂️

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

7 participants