Skip to content
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 socket port 0 error #975

Merged
merged 1 commit into from
Mar 5, 2021
Merged

Fix socket port 0 error #975

merged 1 commit into from
Mar 5, 2021

Conversation

euri10
Copy link
Member

@euri10 euri10 commented Mar 5, 2021

Fixes #974 from possibly a typo introduced in https://github.com/encode/uvicorn/pull/930/files#diff-716652db9a2feab62038753117d5e4fd4294f213f4c7f2edcb47b5788e5209f6L158-R190

I wanted to write a test, natural place seemed to be in

@pytest.mark.asyncio
@pytest.mark.parametrize(
"host, url",
[
pytest.param(None, "http://127.0.0.1:8000", id="default"),
pytest.param("localhost", "http://127.0.0.1:8000", id="hostname"),
pytest.param("::1", "http://[::1]:8000", id="ipv6"),
],
)
async def test_run(host, url):
config = Config(app=app, host=host, loop="asyncio", limit_max_requests=1)
async with run_server(config):
async with httpx.AsyncClient() as client:
response = await client.get(url)
assert response.status_code == 204

unfortunately I can see now way of getting the random assigned port (or the server socket for the same purpose) when we pass a port=0 after it has been created.
do you have an idea @florimondmanca ?

@euri10 euri10 requested a review from florimondmanca March 5, 2021 08:26
Copy link
Member

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

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

👍 OK on the typo.

Not sure about the test, I think there's a way if we access server._server.sockets, but it would need a dedicated test since the run_server() helper is not enough for that.

@selimb
Copy link

selimb commented Mar 5, 2021

It’d be possible, although perhaps not the cleanest, to verify the port by checking the logs?

@euri10
Copy link
Member Author

euri10 commented Mar 5, 2021

yep probably, but I think the better solution would be to effectively check it from the server object and get it in the run_server we use in the test utils.
if you fancy sending a PR, it's welcomed :)
also I just tested this on linux, so hopefully the fix is valid also on windows

@euri10 euri10 merged commit d5d62d4 into encode:master Mar 5, 2021
@euri10 euri10 deleted the port0 branch March 5, 2021 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OSError when --port=0
3 participants