nagios-charmers team mailing list archive

[Stuart Bishop <stuart.bishop@xxxxxxxxxxxxx>]

Review: Approve

Looks good. Some minor issues inline, nothing particularly important. Still needs a ~nagios-charmers review.

Diff comments:

> diff --git a/config.yaml b/config.yaml
> index 4ce0f18..03c270e 100644
> --- a/config.yaml
> +++ b/config.yaml
> @@ -105,7 +105,7 @@ options:
>          type: string
>          default: pageroot@localhost
>          description: |

Can you change this to a > while you are here? The | will render the string badly in the charm store (as preformatted text).

> -            Email address used for the admin pager, used by $ADMINPAGER$ in 
> +            Email address used for the admin pager, used by $ADMINPAGER$ in
>              notification commands.
>      enable_pagerduty:
>          type: boolean
> @@ -168,7 +173,7 @@ options:
>          type: string
>          description: |

Here too, this multi line string will render with normalized white space so > is preferred over |.

>              A string to pass to the Nagios load monitoring command.  Default is
> -            to report warning at 5.0, 4.0 and 3.0 averages, critical at 10.0, 
> +            to report warning at 5.0, 4.0 and 3.0 averages, critical at 10.0,
>              6.0 and 4.0.
>      pagerduty_notification_levels:
>          default: u,c,r
> diff --git a/hooks/upgrade-charm b/hooks/upgrade-charm
> index 19e736c..e154b85 100755
> --- a/hooks/upgrade-charm
> +++ b/hooks/upgrade-charm
> @@ -284,7 +284,8 @@ def update_config():
>  
>  
>  def update_cgi_config():
> -    template_values = {'ro_password': ro_password}
> +    template_values = {'nagiosadmin': hookenv.config('nagiosadmin'),

I think here we want " hookenv.config('nagiosadmin') || 'nagiosadmin' ", so things don't explode when the setting is an empty string. Which should only happen if the user screws up, but it happens.

> +                       'ro_password': ro_password}
>      with open('hooks/templates/nagios-cgi.tmpl', 'r') as f:
>          templateDef = f.read()
>  


-- 
https://code.launchpad.net/~aggkolaitis/nagios-charm/+git/nagios-charm/+merge/374089
Your team Nagios Charm developers is requested to review the proposed merge of ~aggkolaitis/nagios-charm:change_nagios_gui_username into nagios-charm:master.