Review: Needs Fixing
A few comments inline, and while I hate to do this I think the charmhelpers and
this change need to be split. While reviewing it I'm not convinced that this
merge isn't confusing local vs charmhelpers functions. For example
'ingress_address' is defined in this change and I *believe* is only actually
used from charmhelpers.
If I'm just having difficulty understanding and you *do* think the change is
dependent on charmhelpers you can make this MR dependent on the charmhelpers MR
so the changes specific to this bug can be reviewed seperately.
Diff comments:
> diff --git a/hooks/common.py b/hooks/common.py
> index 66d41ec..c2280a3 100644
> --- a/hooks/common.py
> +++ b/hooks/common.py
> @@ -43,6 +43,12 @@ def check_ip(n):
> return False
>
>
> +def ingress_address(relation_data):
> + if 'ingress-address' in relation_data:
> + return relation_data['ingress-address']
> + return relation_data['private-address']
I'm struggling to see this function used anywhere. Why is this being added?
> +
> +
> def get_local_ingress_address(binding='website'):
> # using network-get to retrieve the address details if available.
> log('Getting hostname for binding %s' % binding)
> diff --git a/hooks/install b/hooks/install
> index f002e46..a8900a3 100755
> --- a/hooks/install
> +++ b/hooks/install
> @@ -29,7 +29,7 @@ echo nagios3-cgi nagios3/adminpassword password $PASSWORD |
> debconf-set-selectio
> echo nagios3-cgi nagios3/adminpassword-repeat password $PASSWORD |
> debconf-set-selections
>
> DEBIAN_FRONTEND=noninteractive apt-get -qy \
> - install nagios3 nagios-plugins python-cheetah python-jinja2 dnsutils
> debconf-utils nagios-nrpe-plugin pynag python-apt python-yaml
> + install nagios3 nagios-plugins python-cheetah python-jinja2 dnsutils
> debconf-utils nagios-nrpe-plugin pynag python-apt python-yaml python-enum34
Are we installing python-enum34 to support charmhelpers? I don't see this used
in this diff other than charmhelpers.
>
> scripts/postfix_loopback_only.sh
>
--
https://code.launchpad.net/~aieri/charm-nagios/+git/nagios-charm/+merge/386533
Your team Nagios Charm developers is subscribed to branch charm-nagios: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
