shpool should immediately pick up changes when a new config file is created #29
Assignees
Labels
enhancement
New feature or request
Comments
|
Working on this one now. It is indeed possible to add watches for parent directory (and parent of parent) to get creation events. Note that after a directory being deleted, the watch needs to be re-added. |
Aetf
added a commit
that referenced
this issue
Jun 19, 2024
- Simple merging logic for `Config` - Load config from system-wide locations (fixes #30) - Dynamically pick up changes from newly created config files (fixes #29) - Use `directories-rs` to find user level config directory path A new `ConfigWatcher` is introduced, which opportunistically watches the closest existing parent of target paths and rewatches dynamically accordingly. This way we can get notifications for new file creations in addition to modifications. `ConfigWatcher` also does debounce (100ms by default) to only trigger one config reload for multiple events in a short time period. The core idea of the watcher is as follow: Given a watching target path `/base/sub/config.toml`, a) readd watches if any segment in the path is created/removed; b) reload config only if the event is about the full target path. The actual implementation extends this idea to handle multiple targets. Please refer to `config_watcher.rs` for details and extensive test cases. A few design points: - Notify event handling is done in a separate thread, rather than in notify's internal event handler thread, because the code is more readable and calling unwatch/watch in that thread may lead to deadlock in some cases. (notify-rs/notify#410, notify-rs/notify#463) - For simplicity, `ConfigWatcher` doesn't provide infomation about which exact config file was changed, assuming a reload would need to read all config files anyway.
Aetf
added a commit
that referenced
this issue
Jun 24, 2024
- Simple merging logic for `Config` - Load config from system-wide locations (fixes #30) - Dynamically pick up changes from newly created config files (fixes #29) - Use `directories-rs` to find user level config directory path A new `ConfigWatcher` is introduced, which opportunistically watches the closest existing parent of target paths and rewatches dynamically accordingly. This way we can get notifications for new file creations in addition to modifications. `ConfigWatcher` also does debounce (100ms by default) to only trigger one config reload for multiple events in a short time period. The core idea of the watcher is as follow: Given a watching target path `/base/sub/config.toml`, a) readd watches if any segment in the path is created/removed; b) reload config only if the event is about the full target path. The actual implementation extends this idea to handle multiple targets. Please refer to `config_watcher.rs` for details and extensive test cases. A few design points: - Notify event handling is done in a separate thread, rather than in notify's internal event handler thread, because the code is more readable and calling unwatch/watch in that thread may lead to deadlock in some cases. (notify-rs/notify#410, notify-rs/notify#463) - For simplicity, `ConfigWatcher` doesn't provide infomation about which exact config file was changed, assuming a reload would need to read all config files anyway.
Aetf
added a commit
that referenced
this issue
Jun 24, 2024
- Simple merging logic for `Config` - Load config from system-wide locations (fixes #30) - Dynamically pick up changes from newly created config files (fixes #29) - Use `directories-rs` to find user level config directory path A new `ConfigWatcher` is introduced, which opportunistically watches the closest existing parent of target paths and rewatches dynamically accordingly. This way we can get notifications for new file creations in addition to modifications. `ConfigWatcher` also does debounce (100ms by default) to only trigger one config reload for multiple events in a short time period. The core idea of the watcher is as follow: Given a watching target path `/base/sub/config.toml`, a) readd watches if any segment in the path is created/removed; b) reload config only if the event is about the full target path. The actual implementation extends this idea to handle multiple targets. Please refer to `config_watcher.rs` for details and extensive test cases. A few design points: - Notify event handling is done in a separate thread, rather than in notify's internal event handler thread, because the code is more readable and calling unwatch/watch in that thread may lead to deadlock in some cases. (notify-rs/notify#410, notify-rs/notify#463) - For simplicity, `ConfigWatcher` doesn't provide infomation about which exact config file was changed, assuming a reload would need to read all config files anyway.
Aetf
added a commit
that referenced
this issue
Jun 24, 2024
- Simple merging logic for `Config` - Load config from system-wide locations (fixes #30) - Dynamically pick up changes from newly created config files (fixes #29) - Use `directories-rs` to find user level config directory path A new `ConfigWatcher` is introduced, which opportunistically watches the closest existing parent of target paths and rewatches dynamically accordingly. This way we can get notifications for new file creations in addition to modifications. `ConfigWatcher` also does debounce (100ms by default) to only trigger one config reload for multiple events in a short time period. The core idea of the watcher is as follow: Given a watching target path `/base/sub/config.toml`, a) readd watches if any segment in the path is created/removed; b) reload config only if the event is about the full target path. The actual implementation extends this idea to handle multiple targets. Please refer to `config_watcher.rs` for details and extensive test cases. A few design points: - Notify event handling is done in a separate thread, rather than in notify's internal event handler thread, because the code is more readable and calling unwatch/watch in that thread may lead to deadlock in some cases. (notify-rs/notify#410, notify-rs/notify#463) - For simplicity, `ConfigWatcher` doesn't provide infomation about which exact config file was changed, assuming a reload would need to read all config files anyway.
Aetf
added a commit
that referenced
this issue
Jun 25, 2024
- Simple merging logic for `Config` - Load config from system-wide locations (fixes #30) - Dynamically pick up changes from newly created config files (fixes #29) - Use `directories-rs` to find user level config directory path A new `ConfigWatcher` is introduced, which opportunistically watches the closest existing parent of target paths and rewatches dynamically accordingly. This way we can get notifications for new file creations in addition to modifications. `ConfigWatcher` also does debounce (100ms by default) to only trigger one config reload for multiple events in a short time period. The core idea of the watcher is as follow: Given a watching target path `/base/sub/config.toml`, a) readd watches if any segment in the path is created/removed; b) reload config only if the event is about the full target path. The actual implementation extends this idea to handle multiple targets. Please refer to `config_watcher.rs` for details and extensive test cases. A few design points: - Notify event handling is done in a separate thread, rather than in notify's internal event handler thread, because the code is more readable and calling unwatch/watch in that thread may lead to deadlock in some cases. (notify-rs/notify#410, notify-rs/notify#463) - For simplicity, `ConfigWatcher` doesn't provide infomation about which exact config file was changed, assuming a reload would need to read all config files anyway.
Aetf
added a commit
that referenced
this issue
Jun 25, 2024
- Simple merging logic for `Config` - Load config from system-wide locations (fixes #30) - Dynamically pick up changes from newly created config files (fixes #29) - Use `directories-rs` to find user level config directory path A new `ConfigWatcher` is introduced, which opportunistically watches the closest existing parent of target paths and rewatches dynamically accordingly. This way we can get notifications for new file creations in addition to modifications. `ConfigWatcher` also does debounce (100ms by default) to only trigger one config reload for multiple events in a short time period. The core idea of the watcher is as follow: Given a watching target path `/base/sub/config.toml`, a) readd watches if any segment in the path is created/removed; b) reload config only if the event is about the full target path. The actual implementation extends this idea to handle multiple targets. Please refer to `config_watcher.rs` for details and extensive test cases. A few design points: - Notify event handling is done in a separate thread, rather than in notify's internal event handler thread, because the code is more readable and calling unwatch/watch in that thread may lead to deadlock in some cases. (notify-rs/notify#410, notify-rs/notify#463) - For simplicity, `ConfigWatcher` doesn't provide infomation about which exact config file was changed, assuming a reload would need to read all config files anyway.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently, we use inotify to watch the config file and update it whenever a user makes an edit, but if the config file does not exist we don't set up any watchers. This is sad because it means that users with no setting miss out on their settings getting immediately picked up if they want to make a change.
The simplest fix for this is probably just to autocreate an empty config file on first startup if it does not exist. We could also watch for the file getting created via an inotify watcher on the directory.
The text was updated successfully, but these errors were encountered: