Fix: Save config to disk when restart required (fixes listen address changes) #87
Conversation
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
…changes) When configuration changes require a restart (like listen address changes), the config was not being saved to disk. This caused the changes to be lost after restart, as the server would load the old config file. **Root Cause:** In `ApplyConfig()`, the function returned early when `RequiresRestart` was true, skipping the `SaveConfig()` call that happened later in the function. **Fix:** Moved the `SaveConfig()` operation to execute BEFORE checking if restart is required. This ensures that: 1. Config changes are persisted to disk immediately 2. Restart-required changes take effect on next startup 3. Hot-reloadable changes continue to work as before **Testing:** - Added comprehensive tests in `apply_config_restart_test.go` - Verified listen address changes are saved despite requiring restart - Verified hot-reloadable changes still work correctly - Verified save failures are properly handled - All existing tests pass Fixes issue where Web UI config editor changes to listen address were lost. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
When configuration changes require a restart (like listen address changes),
the config was not being saved to disk. This caused the changes to be lost
after restart, as the server would load the old config file.
Root Cause:
In
ApplyConfig(), the function returned early whenRequiresRestartwas true,skipping the
SaveConfig()call that happened later in the function.Fix:
Moved the
SaveConfig()operation to execute BEFORE checking if restart isrequired. This ensures that:
Testing:
apply_config_restart_test.goFixes issue where Web UI config editor changes to listen address were lost.
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com
Pull Request
Description
Brief description of the changes in this PR.
Testing