Repository navigation
Fix startup URL showing default port instead of configured port - #546
Open
Christian-Sidak wants to merge 2 commits into
Open
Christian-Sidak wants to merge 2 commits into
Christian-Sidak wants to merge 2 commits into
Conversation
When uvicorn is launched via CLI with --port N without also exporting the
PORT environment variable (e.g. using shell default syntax like
${PORT:-3000}), settings.port stays at its default value of 8880 while
the server actually listens on the CLI-specified port. The startup
message then displays the wrong port in the Beta Web Player URL.
Add _get_effective_port(), which inspects sys.argv for --port/--port=/
-p arguments and falls back to settings.port when none is present.
Use it in the lifespan startup message instead of settings.port directly.
Fixes remsky#545
Signed-off-by: Christian Sidak <christian@sentineltech.eu>
Signed-off-by: Christian-Sidak <61099993+Christian-Sidak@users.noreply.github.com>
Owner
|
thanks for catching this! Could we hook uvicorn's "running on" log record instead and print the /web/ link from its args? |
Contributor
Author
|
Good point on sys.argv fragility. Pushed 87e1220: replaced the sys.argv walk with a |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--port Nwithout thePORTenvironment variable being explicitly set (e.g. using shell default expansion like${PORT:-3000}),settings.portstays at its default value (8880) while the server actually listens on the CLI-specified portsettings.portdirectly, so the Beta Web Player URL showed the wrong port_get_effective_port()inmain.pyto inspectsys.argvfor--port/--port=/-parguments, falling back tosettings.portwhen none is found; use it in the startup messageTest plan
_get_effective_port()returnssettings.portwhen no--portarg is present_get_effective_port()returns the CLI value when--port Nis passed_get_effective_port()handles--port=N(equals form)_get_effective_port()handles-p N(short form)_get_effective_port()falls back gracefully when--portvalue is non-integerFixes #545