A new config loader that can monitor config changes #1351
Conversation
Test Results 92 files ± 0 92 suites ±0 10m 33s ⏱️ -4s For more details on these failures, see this check. Results for commit c342115. ± Comparison against base commit ae19c06. This pull request removes 12 and adds 4 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Thanks for creating this PR @AhmedSoliman. The changes look good to me. I had one question concerning the config-watcher thread and whether this could be fused with the debouncer event handler. Apart from this, looking pretty much forward to having a dynamically loadable configuration :-)
| std::thread::Builder::new() | ||
| .name("config-watcher".to_owned()) | ||
| .spawn(move || { | ||
| // It's important that we capture the watcher in the thread, | ||
| // otherwise it'll be dropped and we won't be watching anything! | ||
| let _debouncer = debouncer; | ||
| info!("Configuration watcher thread has started"); | ||
| let mut should_run = true; | ||
| while should_run { | ||
| match rx.recv() { | ||
| Ok(evs) => { | ||
| self.handle_events(evs); | ||
| } | ||
| Err(e) => { | ||
| error!("Cannot continue watching configuration changes: '{}!", e); | ||
| should_run = false; | ||
| } | ||
| } | ||
| } | ||
| info!("Config watcher thread has terminated"); | ||
| }) | ||
| .expect("start config watcher thread"); |
There was a problem hiding this comment.
What is the main purpose of this additional thread? Could we move self.handle_events into the DebounceEventResult handler?
There was a problem hiding this comment.
Yes, this be redesigned to be return a guard that holds the debouncer, but I took the shortest path to get something to work tbh.
A new config loader that can monitor config changes