nagios-charmers team mailing list archive

[Felipe Reyes <felipe.reyes@xxxxxxxxxxxxx>]

On Tue, 2019-12-17 at 05:41 +0000, Stuart Bishop wrote:
> Review: Approve
> 
> This is fine. Some code can be simplified before landing by using
> config.changed('foo') instead of manually comparing config.get('foo',
> None) with config.previous('foo'). But I'm not going to block on this
> since it is matching the existing code.

Oh, .changed(), yeah, I should use that, it will make things easier to
read, I will update the patch.

> 
> Diff comments:
> 
> > diff --git a/hooks/actions.py b/hooks/actions.py
> > index 1593b51..0323ae3 100644
> > --- a/hooks/actions.py
> > +++ b/hooks/actions.py
> > @@ -91,15 +94,22 @@ def update_ppa(service_name):
> >      config = hookenv.config()
> >      new_source = config.get('source')
> >      prev_source = config.previous('source')
> > +
> > +    new_key = config.get('key')
> > +    prev_key = config.previous('key')
> > +
> >      if prev_source is not None and prev_source != new_source:
> >          subprocess.check_call(['add-apt-repository',
> >                                 '--yes', '--remove', prev_source],
> >                                 env=hookenv.env_proxy_settings(['ht
> > tps']))
> > -    add_source(config.get('source'), config.get('key', None))
> > -    apt_update(fatal=True)
> > -    package_list = ["thruk", "pwgen", "apache2-utils"]
> > -    apt_install(packages=package_list, fatal=True)
> > +    if prev_source != new_source or prev_key != new_key:
> 
> This code would be simpler if you used """ config.changed('source')
> or config.changed('key') """
> 
> > +        add_source(config.get('source'), config.get('key', None))
> > +        apt_update(fatal=True)
> > +
> > +    pkgs_to_install = filter_installed_packages(PACKAGE_LIST)
> > +    if pkgs_to_install:
> > +        apt_install(packages=pkgs_to_install, fatal=True)
> >  
> >  
> >  def update_status(service_name):
> > -   hookenv.status_set('active', 'ready')
> > +    hookenv.status_set('active', 'ready')
> 
> 
-- 
Felipe Reyes
Software Sustaining Engineer @ Canonical
# Email: felipe.reyes@xxxxxxxxxxxxx (GPG:0x9B1FFF39)
# Launchpad: ~freyes | IRC: freyes


https://code.launchpad.net/~freyes/thruk-agent-charm/+git/thruk-agent-charm/+merge/376486
Your team Nagios Charm developers is requested to review the proposed merge of ~freyes/thruk-agent-charm:lp1829681 into thruk-agent-charm:master.