-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 http crate #3208
chore: update http crate #3208
Conversation
I'm curious if you have any particular motive to get this merged. From my PoV, the Actix Web project doesn't have any real need to do it. |
Thanks for your feedback! I don't have any particular motive other than having that dependency up to date, so other crates that have |
Not really sure how that happened but:
whereas the rest of the actix universe is depending on 0.2. It is a bit unfortunate that one set of actix crates pulls in one dependency in two different versions. |
Context for that particular change: actix/actix-net#508 |
I think switching to and using a 1.0 version is always a good venture due to the implied stability guarantees. |
Is there anything blocking this PR? |
One use case is passing header values extracted from actix's request to other libraries, such as reqwest 0.12 . |
Inevitably security support for http 0.2.x will cease long before security support for http 1.x does. |
If I understand correctly from the milestone, the maintainers are waiting for the v5.0 release to merge this? Hopefully this is done sooner than later, as there is 0 reason for any actively maintained library to still be using |
There's no security risk of staying on v0.2. It's my opinion that it's not worth a breaking change just for this. The ecosystem churn of releasing Actix Web v5.0 is vast; far outweighing any potential benefits of sticking with this older version until enough good reasons exist to break everyone. |
I can see holding this off until the next major version bump since it is, as you said, a breaking change. The thing that drew me to this issue is the one @juchiast mentioned: I'm unable to upgrade the reqwest dependency in one of my projects to 0.12.x without doing a bunch of workarounds. As the documentation for the crate says says:
With the crate's status as standard to be used by client and server crates, and with many of those crates updating to use the 1.x version, the incompatibility issues will only increase as time goes on. |
Sorry, but compatibility with the I disagree that |
fwiw i already did the reqwest 0.12 upgrade in pict-rs and the only thing I needed to add was a conversion between http1 and http02 status codes, which I did with this code: pub(crate) fn to_actix_status(status: reqwest::StatusCode) -> actix_web::http::StatusCode {
actix_web::http::StatusCode::from_u16(status.as_u16()).expect("status codes are always valid")
} |
I'm a little late to discussion.
From a p.o.v. of Actix (maintainer) indeed there might not be much motivation. From an Actix user however there are. Currently we have two versions of the same crate in our dependency tree due to Actix, the leads to (among other this) larder binary and increased build time. This also leads to slight incompatibility between since as highlighted by @asonix (#3208 (comment)).
This is totally a fair point of view. And thank you for not creating churn for no reason (it's one of my pet peeves). Also a note on how this pr is implemented; it won't work. If a single dependency decides to enable the |
Depends on actix/actix-net#508
PR Type
Updates http crate
PR Checklist
Overview