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

Output correct port if port=0, refs #530 #531

Merged
merged 2 commits into from Dec 19, 2019
Merged

Conversation

simonw
Copy link
Contributor

@simonw simonw commented Dec 19, 2019

Should fix #530

@simonw
Copy link
Contributor Author

simonw commented Dec 19, 2019

Weird: https://travis-ci.org/encode/uvicorn/jobs/627147183

Test fails but only under Python 3.6:

tests/supervisors/test_statreload.py .F                                  [100%]

=================================== FAILURES ===================================

______________________________ test_should_reload ______________________________

tmpdir = local('/tmp/pytest-of-travis/pytest-0/test_should_reload0')
    @pytest.mark.skipif(
        sys.platform.startswith("win"),
        reason="Skipping reload test on Windows, due to low mtime resolution.",
    )
    def test_should_reload(tmpdir):
        update_file = Path(os.path.join(str(tmpdir), "example.py"))
        update_file.touch()

        working_dir = os.getcwd()

        os.chdir(str(tmpdir))

        try:
            config = Config(app=None, reload=True)
            reloader = StatReload(config, target=run, sockets=[])
            reloader.signal_handler(sig=signal.SIGINT, frame=None)
            reloader.startup()
    
            assert not reloader.should_restart()
            update_file.touch()
>           assert reloader.should_restart()
E           assert False
E            +  where False = <bound method StatReload.should_restart of <uvicorn.supervisors.statreload.StatReload object at 0x7f60701cdb00>>()
E            +    where <bound method StatReload.should_restart of <uvicorn.supervisors.statreload.StatReload object at 0x7f60701cdb00>> = <uvicorn.supervisors.statreload.StatReload object at 0x7f60701cdb00>.should_restart

@@ -428,6 +428,9 @@ def run(self, sockets=None):
except OSError as exc:
logger.error(exc)
sys.exit(1)
port = config.port
if port == 0:
port = server.sockets[0].getsockname()[1]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will server.sockets only ever have a single item in the list here? I think so but I'm not certain.

@tomchristie
Copy link
Member

Thanks @simonw.

Test fails but only under Python 3.6

Righty, that's actually a flaky test right there, since it's dependant on the time resolution and how quickly the .touch takes effect.

Let me take a look at resolving that, then once that's in you can merge master back into this.

@tomchristie
Copy link
Member

Righty, worth pulling master into this now, which ought to resolve the issue.

@simonw
Copy link
Contributor Author

simonw commented Dec 19, 2019

That fixed it!

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.

Correctly output selected port if port=0
2 participants