-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Implement the OptionalFromRequestParts trait for the Host extractor #3177
base: main
Are you sure you want to change the base?
Conversation
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.
LGTM thanks :)
@jplatte feel free to review :) |
I don' t hink we want I think it would be better to surface the parsing errors and return Others' opinions might differ and we can go ahead with this in that case, if not I'm not sure where that would leave this PR. We could merge it and change it along with the other extractor or just wait with it. |
My availability is limited for the next two weeks or so, but I'll get to reviewing it some time next month. |
@mladedav the see also #3177 (comment) 😉 |
Yeah, I get that it doesn't discard more errors than the other extractor, but that doesn't change that it does discard them. I appreciate we want the two to work as similarly as possible, but I'm not sure that this addition is really needed then. The only reason I can see why people want this is because people are used to I generally don't think we should have any |
Fixes: #3170
Motivation
To allow for
Option<Host>
extractors we need to implement the OptionalFromRequestParts trait for it.Solution
Implement OptionalFromRequestParts for the Host Extractor