Skip to content

Handle default ports in WSGITransport - #1469

Merged
florimondmanca merged 3 commits into
encode:masterfrom
abersheeran:patch-2
Feb 16, 2021
Merged

Handle default ports in WSGITransport#1469
florimondmanca merged 3 commits into
encode:masterfrom
abersheeran:patch-2

Conversation

@abersheeran

@abersheeranabersheeran commented Feb 16, 2021

Copy link
Copy Markdown
Contributor

https://www.python.org/dev/peps/pep-3333/#environ-variables

SERVER_NAME, SERVER_PORT
------------------------------------------------------------
When HTTP_HOST is not set, these variables can be combined to determine a default. See the URL Reconstruction section below for more detail. SERVER_NAME and SERVER_PORT are required strings and must never be empty.

Fixed #1468

https://www.python.org/dev/peps/pep-3333/#environ-variables
> SERVER_NAME, SERVER_PORT
> When HTTP_HOST is not set, these variables can be combined to determine a default. See the URL Reconstruction section below for more detail. SERVER_NAME and SERVER_PORT are required strings and must never be empty.
@abersheeranabersheeran changed the title Fix #1468environ["SERVER_PORT"] can't be "None"Feb 16, 2021
@abersheeranabersheeran changed the title environ["SERVER_PORT"] can't be "None"WSGI environ["SERVER_PORT"] can't be "None"Feb 16, 2021

@florimondmancaflorimondmanca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Also, is there a unit test we could add in test_wsgi.py about this?

Comment threadhttpx/_transports/wsgi.py Outdated
@florimondmancaflorimondmanca added the bug Something isn't working label Feb 16, 2021
@florimondmanca

Copy link
Copy Markdown
Contributor

@abersheeran I went ahead and pushed some commits for the modifications to get this PR in shape, hope you don't mind.

@florimondmancaflorimondmanca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for this contribution!

We're not sure yet when 0.17.0 will be released (the team needs to circle back on that), but once merged this would at least be available for git installation.

@florimondmancaflorimondmanca changed the title WSGI environ["SERVER_PORT"] can't be "None"Handle default ports in WSGITransportFeb 16, 2021
@florimondmanca
florimondmanca merged commit 02a692a into encode:masterFeb 16, 2021
@abersheeran

Copy link
Copy Markdown
ContributorAuthor

Thank you for your quick response~

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugSomething isn't working

Development

Successfully merging this pull request may close these issues.

environ["SERVER_PORT"] can't be "None"

2 participants

@abersheeran@florimondmanca