> Code looks fine, but there are some inline comments that will need to be > addressed. > > Is this change useful as is? Looking at the examples, extra binaries are > needed, which makes me think adding in the extra contacts is a job for a > subordinate charm. The charm has no support for installing extra deb packages > or snaps, and I suspect for real world deployments it is expected that someone > manually copies the custom notification binaries into place (in which case, > why bother with extending the charm at all when we can just cowboy the config > changes at the same time we inject the needed binaries). Will need a ~nagios- > charmer to help here.
I applied some fixes to the code as requested. Regarding usability, the notification could either be some external tool that needs installation (which is the most likely scenario), or a very simple `curl` command. In that sense, I believe it is acceptable to offer that functionality via the charm. The alternative (hi-jacking the config when installing the external tool) is not a viable option, because updating the Nagios configuration/charm version will overwrite those changes. Actually, this is what is happening right now, and which is why I started working on this. That said, I agree that we need the opinion of a ~nagios-charmer -- https://code.launchpad.net/~aggkolaitis/nagios-charm/+git/nagios-charm/+merge/374074 Your team Nagios Charm developers is requested to review the proposed merge of ~aggkolaitis/nagios-charm:extra_contacts into nagios-charm:master. -- Mailing list: https://launchpad.net/~nagios-charmers Post to : [email protected] Unsubscribe : https://launchpad.net/~nagios-charmers More help : https://help.launchpad.net/ListHelp
