-
Notifications
You must be signed in to change notification settings - Fork 643
Use http types from http
crate
#2389
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
Conversation
I'm also pushing this to staging for some testing there. The deploy should finish within the next 15 minutes. (The deployed SHA is available in the site metadata which can be used to confirm staging still points to the latest commit in this PR, currently |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Feel free to r=me if it's ready.
req.header( | ||
// Needed for the error message we generate | ||
// FIXME: Simplify once we hit `conduit-test 0.9.0-alpha.2` | ||
header::HeaderName::from_lowercase(b"x-request-id").unwrap(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be nice the http crate had that header..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: hyperium/http#411
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Thanks for opening the upstream PR.
@bors r=JohnTitor |
📌 Commit 6985639 has been approved by |
☀️ Test successful - checks-travis |
In the upstream conduit crates, I've dropped the custom HTTP related types in favor of types from the
http
crate. This reduces the amount of code to maintain upstream and generally provides a cleaner API, such as namedStatusCode::*
constants instead of naked integers.This should also make it easier and more efficient to support both
civet
andhyper
as web servers, as they (and our middleware stack) now use the same types and avoid unnecessary conversions.r? @JohnTitor