Code review comment for lp:~benji/charms/precise/juju-gui/bug-1284088

Thanks for the good review. Read all the comments before replying
because there is a twist ending.

Sincerely,
M. Night Shyamalan

https://codereview.appspot.com/76860047/diff/1/scripts/charmsupport/nrpe.py
File scripts/charmsupport/nrpe.py (right):

https://codereview.appspot.com/76860047/diff/1/scripts/charmsupport/nrpe.py#newcode113
scripts/charmsupport/nrpe.py:113: def service_file_name(self,
nagios_context, hostname):
On 2014/03/19 17:42:04, frankban wrote:
> nagios_context seems unused in this method.

You are exactly right. Given the complete lack of tests and the fact
that this is a dead library plus the extensive testing I've done of the
code as-is, I'm going to leave it be (unless you object strenuously).

https://codereview.appspot.com/76860047/diff/1/scripts/charmsupport/nrpe.py#newcode130
scripts/charmsupport/nrpe.py:130: nrpe_service_file =
'{}/service__{}_check_{}.cfg'.format(
On 2014/03/19 17:42:04, frankban wrote:
> The nrpe_service_file is retrieved using the external method in the
next line,
> so I think we can safely remove this one.

Good catch. Again, I am loathe to touch the known-working code, even to
do something this simple.

https://codereview.appspot.com/76860047/diff/1/scripts/update-nrpe.py
File scripts/update-nrpe.py (right):

https://codereview.appspot.com/76860047/diff/1/scripts/update-nrpe.py#newcode25
scripts/update-nrpe.py:25: remove_nrpe_check()
On 2014/03/19 17:42:04, frankban wrote:
> So we always remove checks and then add them (or remove them again)
based on the
> hook name. Is the second removal needed?

Darn, this is bad code. This really is a stopper, so I'm going to
retract what I said before about changing the code and implement the two
suggestions you made as well as remove this line and then re-test.

https://codereview.appspot.com/76860047/