Cleaner Pid observable code #12702
Cleaner Pid observable code #12702
Conversation
platform/plugins/pid/PidService.ts
Outdated
| } | ||
|
|
||
| // Otherwise we return an observable that writes the pid when | ||
| // subscribed to and deletes it when unsubscribed (e.g. if new config | ||
| // is received or if `stop` is called below.) | ||
|
|
||
| return new Observable<PidFile | void>(observable => { | ||
| return new Observable(observable => { |
There was a problem hiding this comment.
nit: observable argument is not used, so we can get rid of it.
| .switchMap(config => { | ||
| if (config === undefined) { | ||
| // If there is no pid config we return an observable that does nothing | ||
| return new Observable(noop); | ||
| } | ||
|
|
||
| // Otherwise we return an observable that writes the pid when | ||
| // subscribed to and deletes it when unsubscribed (e.g. if new config |
There was a problem hiding this comment.
question: does e.g. if new config is received mean I can change pid file path on-the-fly and old file will be deleted and new one created? Is it supposed to work at the moment (didn't manage to see that behaviour)?
There was a problem hiding this comment.
Yep, it's doing that now:
I added some newlines here, and performed the sighup in another window. But just change the config, then sighup the process (kill -1 it), and it should re-apply the config. I specified logging.level: debug first in the kibana config to see the debug logs.
There was a problem hiding this comment.
then sighup the process (kill -1 it)
Ah, that the reason, I didn't do that (for some reason I thought we watch the config file for changes all the time), thanks :)
There was a problem hiding this comment.
Watching is "less safe" as you can be in intermediate states when it triggers. E.g. if you're moving stuff around in the config and hit save at some point (e.g. my IDE that saves whenever I cmd-tab away). It's better to let the user actively notify that the config is ready.
The code became simpler when moving the config stuff into the
switchMap. Otherwise just some minor cleanups.