fix(cli): pyfly run resolves the port like the application does - #165
Merged
Merged
Conversation
`pyfly run` chose the port from the --port flag, then a raw read of pyfly.yaml, then 8080. That bypassed the configuration system entirely: the relaxed-binding override PYFLY_SERVER_PORT, the -D server.port=9000 example advertised by --define's own help text (which becomes exactly that variable), and the profile overlays were all inert for the bind port, while the application read the same key through Config and believed the overridden value. Found starting a service whose default port was taken: the server bound 8080 next to the other process and nothing listened where the operator asked. The CLI now resolves the port with the same precedence as Config: PYFLY_SERVER_PORT first (a non-integer is a usage error, not a silent 8080), then the merged configuration for the active profiles, and only then the raw file. Seven tests cover the file, the env override with and without a file, the -D path end to end, the profile overlay and the bad value.
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
1 participant
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.
What was wrong
pyfly runpicked the bind port as--port→ a raw read ofpyfly.yaml→8080. That bypassed the configuration system, so three documented ways of setting the port were silently inert for the CLI:PYFLY_SERVER_PORT=8090(the relaxed-binding overrideConfig.get()honours for every key);-D server.port=9000— the very example in--define's help text, which_build_launch_envturns into that same variable;pyfly-{profile}.yamloverlays (andconfig/pyfly.yaml).The application process reads the same key through
Configand believed the overridden value, so the banner/log said one port and the server bound another. Found while starting a service on a machine where 8080 was already taken: the server bound 8080 next to the other process and nothing listened where the operator asked (PYFLY_MANAGEMENT_SERVER_PORTworked, which made it look like a per-key bug).Fix
_read_port_from_config()resolves with the same precedence asConfig:PYFLY_SERVER_PORTfirst (a non-integer is aclick.BadParameter, not a silent 8080), thenConfig.from_sources(cwd, active_profiles=…, load_defaults=False).get("pyfly.server.port"), and only then the raw file as a last resort.--portstill wins over everything.Tests
TestReadPortFromConfig(7 cases): no file, file, env over file, env without file,-D server.port=9000end to end through_build_launch_env, profile overlay over base, bad env value.ruff,ruff format,mypy --strict,tests/cli(300) and the unit suite (4972 passed) are green locally.