-
Notifications
You must be signed in to change notification settings - Fork 63
PBS: Reload the config file when it changes #409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| // Set up the filesystem watcher for the config file | ||
| let mut watcher: RecommendedWatcher; | ||
| if config_path.to_str() != Some("") { | ||
| let state_for_watcher = state.clone(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would add a couple of logs:
- that we start the file watcher
- when trying to reload the config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, done in 8d77531.
crates/pbs/src/service.rs
Outdated
| let config_path_for_watcher = config_path.clone(); | ||
| watcher = RecommendedWatcher::new( | ||
| move |result: Result<Event, Error>| { | ||
| let event = result.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why unwrap here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's just how their example usage worked, but I replaced it with a match in 8d77531.
This closes the first request in #382 by automatically reloading the config for the PBS service as though
/reloadwere called whenever the config file changes. There are a few ways to do this, but I settled on propagating the path from the config loader instead of making it a field within thePbsConfig, because it didn't feel like a good fit for that.Note that the Signer service doesn't do hot-reloading, because it's a much different setup that will require more planning. That might be OBE during the config revamp anyway so I'm leaving it out for now.