-
-
Notifications
You must be signed in to change notification settings - Fork 932
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
Fix port issue on host-based routing #1322
Conversation
If you accept this change, then we will need to update info in the documentation (I added a note in #1313). I can commit to this pull request, or create a new one. What should I do? |
Hey thanks for that. Surely a unit test can be added! |
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
Yes, I think we can update the doc in the same PR since they are related. |
I think that in general the pull request is ready. But I'm not sure about the quality of the wording in the note |
These errors are not caused by code, is it? |
Probably Github's issue:
|
It looks like is running now. I've relaunch the pipeline. |
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. Thank you for this.
Host.host_regex
is only used to check "Host" header: (routing.py:432
)However, when generating
Host.host_regex
, the port remained: (routing.py:149
)I added one small fix. Now the port is also removed from
Host.host_regex
UPD: Before this fix, the code from #1317 example returned
Not found
, and afterHello World