Add ability to watch config for changes #207
Conversation
|
I was thinking of a different workflow here, but I don't have strong opinions. The way, say, This would interact appropriately with |
|
@dalcde Great idea. I'll definitely update the code to use signals. |
This will not be functional until some version of matrix-org/matrix-appservice-bridge#207 is merged, since there is currently no way to access, load and validate the new config file without reimplementing the logic of matrix-appservice-bridge.
|
This has been done. |
| const newConfig = this.loadConfig(configFilename); | ||
| // onConfigChanged is checked above, but for Typescript's sake. | ||
| if (this.opts.onConfigChanged) { | ||
| this.opts.onConfigChanged(newConfig); |
There was a problem hiding this comment.
Do you want to update the this.bridgeConfig variable here?
In real life, only some config variables would be hot-reloadable. I would expect that if the new config edits other variables, the bridge should reject the new config file and retain the old one. We can make onConfigChanged return a boolean to indicate whether the new config file is accepted, and act based on that.
There was a problem hiding this comment.
Nope, I don't want to update the config from underneath bridges because it could cause problems. For instance, certain bits of the IRC bridge cannot be reloaded so we wouldn't want to change those variables.
It will be up to the bridge developer to listen for the callback, and explicitly apply changes to the config inside the bridge. It might be a bit more faff, but it's safer.
|
One issue I envsion is that bridgeConfig is a read-only variable, so it
is impossible to reassign it to the new config.
|
src/components/cli.ts
Outdated
| // onConfigChanged is checked above, but for Typescript's sake. | ||
| if (this.opts.onConfigChanged) { |
There was a problem hiding this comment.
Not just for TypeScript's sake. We're copying the opts object by reference.
Any code in this class or the bridge could have re-set the value of opts.onConfigChanged before SIGHUP was received.
This allows bridge developers to listen for config changes, which can be bubbled up to the bridge logic to support small changes without restarting the whole bridge. This is useful for things like the irc bridge, where restarts are costly.
I've also added some minimal Cli tests to ensure that config behaviour continues to work as expected.