Announce the web app's URL only once its server is bound - #179
Merged
Conversation
examples/web_app.py printed the address before anything was listening on it, and built that address from its own copy of FISHE_WEB_HOST/FISHE_WEB_PORT rather than from the server. Both ordinary startup failures - a misspelled port and a busy one - were therefore preceded by a confident sentence naming an address that would never answer, since the value is not converted there at all: UserInterfaceFactory's WEB branch does that, and WebUserInterface binds, both after the print. web/serve.py was given the opposite ordering in #177, so the two web entry points disagreed about it. The ordering cannot simply be swapped there: FishE.__init__ builds the front-end and then blocks in the save-file manager, so control never comes back to main() to say what was bound. The announcement moves to WebUserInterface instead, immediately after _bindServer returns, and names the socket's own address - so a caller that asked for an ephemeral port is told the one it actually got. The entry point now announces nothing of its own. The "8000" default and the two variables that move it move to webUserInterface as DEFAULT_HOST/DEFAULT_PORT and resolveAddressFromEnvironment(), which the factory branch and the constructor now share; the example no longer reads the environment at all. web/serve.py keeps its own 8080 default, deliberately, as its comment already explains. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
Self-review rubric (scored against the diff and command output, not judgement):
Two observations that were judged rather than measured, folded in here rather than left inline:
This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener). drafted by Claude on behalf of Daniel Stephenson |
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
examples/web_app.pyprintedhttp://{host}:{port}before anything was listening on it, from its own copy ofFISHE_WEB_HOST/FISHE_WEB_PORT. Both ordinary startup failures — a misspelled port and a busy one — were therefore preceded by a confident sentence naming an address that would never answer, since neither the conversion (UserInterfaceFactory's WEB branch) nor the bind (WebUserInterface._bindServer) happens until after that print.web/serve.pywas given the opposite ordering in Name FISHE_WEB_PORT when a web port is busy or misspelled #177, so the two web entry points disagreed about it.FishE.__init__builds the front-end and then blocks in_selectSaveFile(), so control never returns tomain()to say what was bound. The announcement is instead made byWebUserInterfaceimmediately after_bindServerreturns, and names the socket's own address — so a caller that asked for an ephemeral port is told the port it actually got. The entry point now announces nothing of its own, and a failed bind is no longer preceded by a URL."8000"default and the two variables that move it are now stated once, inwebUserInterface(DEFAULT_HOST/DEFAULT_PORTandresolveAddressFromEnvironment()), and shared by the factory branch and the constructor; the example no longer reads the environment at all.web/serve.pykeeps its own8080default deliberately, as the comment there already explains.WebUserInterfacewithstart_server=False, so it has no address and announces nothing.Test plan
python3 -m pytest(withSDL_VIDEODRIVER/SDL_AUDIODRIVER=dummy) — 829 passedresolveAddressFromEnvironment()covers the default, the override, and the misspelled-portValueErrortests/examples/test_web_app.py: the entry point builds the WEB front-end and prints no address of its ownblack --checkon the changed filesDeferred this cycle
No other issue was open at triage time, so nothing was deferred.
Closes #178
This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).
drafted by Claude on behalf of Daniel Stephenson