Repository navigation
Config correctness: rename references, atomic writes, no silent settings, daemon port - #94
Merged
Merged
Conversation
- `pyrolab rename` (#67) renames an entity together with every reference to it (service daemon/nameservers, daemon nameservers, autolaunch, including the implicit "default" daemon), keeps entry order, refuses an existing name unless --force, reports what changed, says "Daemon/Service ... not found" correctly, and notes when a persistent nameserver's storage file will change. New configure.rename_entity() does the work. - Atomic writes (#69): update_config, export_config, save_config, `config export`, and the lockfile all go through the new utils.atomic_write_text (temp file, fsync, os.replace), so an interrupted write leaves the previous file intact. Fixes the "File does not:" message. - No silent settings (#72): the config models are plain pydantic models instead of BaseSettings, which filled unset fields from same-named environment variables (PORT=8080 moved every daemon). Unknown keys stay errors (now covered by tests). update_pyro_config logs what it applies and warns about anything it can't, unless the model declares the field as used elsewhere (NOT_PYRO_OPTIONS). - Daemon placement (#47): host, port, unixsocket, nathost, and natport are passed to the daemon's constructor, so `port` and `unixsocket` finally take effect. - PyroLabConfiguration names its nameservers on construction, so a config loaded with from_file() no longer computes storage paths like "ns_.sql". Closes #47 Closes #67 Closes #69 Closes #72 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
reload restarted only what was running under names still in the configuration, so an autolaunched nameserver or daemon that had been renamed (or added to autolaunch) stayed stopped until started by hand. reload now also starts every autolaunch entry, nameservers first, and counts them in its result, so it matches a fresh `pyrolab up` plus anything started by hand. The rename hint now says plain `reload` is enough for autolaunched entities. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Finishes hardening configuration handling. Closes #47, #67, #69, #72. Targets
masterand depends on nothing else.pyrolab renameupdates references (#67)daemonandnameservers, each daemon'snameservers, and theautolaunchlists. That includes services that use the implicitdefaultdaemon. The output lists what changed, for exampleUpdated references in: service 'sample.echo', daemon 'lockable', autolaunch.. Since Make daemon startup reliable: validated config, atomic lockfile, waiting up #92, the command refused these renames instead.--forceis given. With--force, the existing entity is replaced and duplicate references collapse into one. Previously the existing entry was silently destroyed.Daemon 'x' not found.andService 'x' not found.replace the copy-pasted "Nameserver not found". Renaming to the same name does nothing.sqlordbmstorage, the command notes that its database file name changes with its name, so the file needs moving to keep existing registrations.autolaunch, plainpyrolab reloadis enough (see below). For one that isn't, the message gives the exactpyrolab start <kind> <new>command, sincereloadcan't tell that the renamed entity is the same one.configure.rename_entity(), which returns the validated, renamed config plus the list of changes.Atomic config writes (#69)
update_config,export_config,GlobalConfiguration.save_config,pyrolab config exportand the lockfile all use the newutils.atomic_write_text(). It writes a temporary file in the same directory,fsyncs it, then swaps it in withos.replace. If anything fails, the previous file is left intact and no temporary file remains.No silently ignored or injected settings (#72)
BaseSettings, which fill any field the YAML leaves out from an environment variable with the same name, ignoring case. WithPORT=8080in the environment, every daemon would bind port 8080;HOSTandSERVERTYPEwould silently change those settings too. The models are now plainBaseModels, so the config file is the only source. They keep the same strictness: unknown keys are rejected and defaults are validated. Nothing in PyroLab used environment variables this way on purpose.BaseSettingsalready rejected them. They're still rejected, and regression tests now cover several misspellings.update_pyro_configreports what it does: it logs the settings it applies at debug level, and logs a warning for any it can't apply. The exception is fields a model lists inNOT_PYRO_OPTIONSbecause they're used elsewhere, such asstorage,broadcast,moduleandport.ServiceConfigurationno longer inherits the Pyro-settings mixin. Nothing called it, and services have no Pyro settings.Daemon
portandunixsockettake effect (#47)DaemonRunner.setup_daemonnow passeshost(withpublicresolved),port,unixsocket,nathostandnatportstraight to the daemon. Before, they were only applied throughPyro5.config, which has noPORTorUNIXSOCKETsetting, so a configured port was ignored.natportis passed only together withnathost, because Pyro requires both or neither.reloadstarts autolaunch entriesreloadused to restart only what was already running, under names still in the config. Now it also starts everyautolaunchentry, nameservers first, and counts them toward its success result. After a reload you get what a freshpyrolab upwould start, plus anything you'd started by hand. This also means an autolaunched entity you'd stopped deliberately comes back on reload.This came out of the rename work. Renaming an autolaunched nameserver used to leave it stopped after
reload, which an end-to-end run showed. Nowrenamefollowed byreloadbrings it back under its new name, and the daemon re-registers with it right away.Also fixed
PyroLabConfigurationnow names its nameservers whenever it's constructed. Before, onlyGlobalConfiguration.load_config()did, so a config loaded withfrom_file()computed storage paths likens_.sql. The rename storage note exposed this.Checks run
master, the new rename, atomic-write, placement, environment-variable, logging and nameserver-name tests all fail.PORT=12345set in the environment, a daemon configured with its own port listened on that port;rename nameserver testns labnsrewrote all three references;reloadand thenstart nameserver labns, the daemon registered withlabnson its next retry;downleft no processes behind.🤖 Generated with Claude Code