Code review comment for lp://staging/~mbruzek/charms/trusty/apache2/nagios_extra_check_https

we went with a full new config because when you have many checks you really
care about being able to set sane descriptions for them, and finding a
format for that in one string I think would get unwieldy. Honestly we need
keep the original one for backward compatible/the simple case

On Thu, Oct 29, 2015 at 6:30 PM, Kevin W Monroe <email address hidden>
wrote:

> Hey @mbruzek and @pedronis,
>
> The new monitoring section of the README is fantastic. Excellent job!
>
> However, I'm curious if we could implement this extra_http_checks
> differently. Rather than create a new config opt that takes yaml (which is
> difficult to type into the cli and juju-gui), could we make the existing
> nagios_check_http_params take multiple args? If so, it seems like the
> functionality from this MP could be achieved with:
>
> juju set apache2 nagios_check_http_params="-H hostA -e '200 OK'; -H hostB
> -e '200 OK'"
>
> The update_nrpe_checks() in hooks.py would then split
> nagios_check_http_params on a semicolon (or whatever delimiter makes sense)
> and call check_http on each of the splits. The README would also need to be
> updated to let users know that nagios_check_http_params could take multiple
> check params.
>
> To me this is less confusing than having 2 config opts that are both
> passed on to check_http. Thoughts?
>
> As an aside, when working through this review, I noticed 'make test'
> failed. I know you didn't introduce this failure, but I opened a bug in
> case you're interested in fixing :)
>
> https://bugs.launchpad.net/charms/+source/apache2/+bug/1511474
> --
>
> https://code.launchpad.net/~mbruzek/charms/trusty/apache2/nagios_extra_check_https/+merge/275095
> You are reviewing the proposed merge of
> lp:~mbruzek/charms/trusty/apache2/nagios_extra_check_https into
> lp:charms/trusty/apache2.
>