nagios-charmers team mailing list archive

[Drew Freiberger <drew.freiberger@xxxxxxxxxxxxx>]

Review: Needs Fixing

a number of black concatenated line strings still hanging out.  see inline comments for locations.

Diff comments:

> diff --git a/hooks/monitors_relation_changed.py b/hooks/monitors_relation_changed.py
> index 5e4f664..d541f2a 100755
> --- a/hooks/monitors_relation_changed.py
> +++ b/hooks/monitors_relation_changed.py
> @@ -52,20 +52,19 @@ def _prepare_relation_data(unit, rid):
>      relation_data = relation_get(unit=unit, rid=rid)
>  
>      if not relation_data:
> -        msg = (
> -            'no relation data found for unit {} in relation {} - '
> -            'skipping'.format(unit, rid)
> +        msg = "no relation data found for unit {} in relation {} - " "skipping".format(

remove " "

> +            unit, rid
>          )
>          log(msg, level=DEBUG)
>          return {}
>  
> -    if rid.split(':')[0] == 'nagios':
> +    if rid.split(":")[0] == "nagios":
>          # Fake it for the more generic 'nagios' relation
> -        relation_data['target-id'] = unit.replace('/', '-')
> -        relation_data['monitors'] = {'monitors': {'remote': {}}}
> +        relation_data["target-id"] = unit.replace("/", "-")
> +        relation_data["monitors"] = {"monitors": {"remote": {}}}
>  
> -    if not relation_data.get('target-address'):
> -        relation_data['target-address'] = ingress_address(unit=unit, rid=rid)
> +    if not relation_data.get("target-address"):
> +        relation_data["target-address"] = ingress_address(unit=unit, rid=rid)
>  
>      for key in REQUIRED_REL_DATA_KEYS:
>          if not relation_data.get(key):
> @@ -73,9 +72,8 @@ def _prepare_relation_data(unit, rid):
>              # the relation at first (e.g. gnocchi). After a few hook runs,
>              # though, they add the key. For this reason I think using a logging
>              # level higher than DEBUG could be misleading
> -            msg = (
> -                '{} not found for unit {} in relation {} - '
> -                'skipping'.format(key, unit, rid)
> +            msg = "{} not found for unit {} in relation {} - " "skipping".format(

remove " "

> +                key, unit, rid
>              )
>              log(msg, level=DEBUG)
>              return {}
> diff --git a/tests/functional/test_config.py b/tests/functional/test_config.py
> index b64f8a6..8f83bdd 100644
> --- a/tests/functional/test_config.py
> +++ b/tests/functional/test_config.py
> @@ -108,57 +113,63 @@ async def test_web_interface_with_ssl(auth, unit, ssl):
>      assert r.status_code == 200, "HTTPs Admin login failed"
>  
>  
> -@pytest.mark.usefixtures('extra_config')
> +@pytest.mark.usefixtures("extra_config")
>  async def test_extra_config(auth, unit):
> -    host_url = "http://%s/cgi-bin/nagios3/status.cgi?"; \
> -              "hostgroup=all&style=hostdetail" % unit.u.public_address
> +    host_url = (
> +        "http://%s/cgi-bin/nagios3/status.cgi?";
> +        "hostgroup=all&style=hostdetail" % unit.u.public_address
> +    )
>      r = requests.get(host_url, auth=auth)
> -    assert 'extra_config' in r.text, "Nagios is not monitoring extra_config"
> +    assert "extra_config" in r.text, "Nagios is not monitoring extra_config"
>  
>  
>  async def test_live_status(unit, livestatus_path, file_stat):
>      stat = await file_stat(livestatus_path, unit.u)
> -    assert stat['size'] == 0, (
> -        "File %s didn't match expected size" % livestatus_path
> -    )
> +    assert stat["size"] == 0, "File %s didn't match expected size" % livestatus_path
>  
>  
>  async def test_pager_duty(unit, enable_pagerduty, file_stat):
>      stat = await file_stat(enable_pagerduty, unit.u)
> -    assert stat['size'] != 0, (
> -        "Directory %s wasn't a non-zero size" % enable_pagerduty
> -    )
> -    stat = await file_stat('/etc/nagios3/conf.d/pagerduty_nagios.cfg', unit.u)
> -    assert stat['size'] != 0, "pagerduty_config wasn't a non-zero sized file"
> +    assert stat["size"] != 0, "Directory %s wasn't a non-zero size" % enable_pagerduty
> +    stat = await file_stat("/etc/nagios3/conf.d/pagerduty_nagios.cfg", unit.u)
> +    assert stat["size"] != 0, "pagerduty_config wasn't a non-zero sized file"
>  
>  
>  async def test_extra_contacts(auth, unit, set_extra_contacts):
> -    contancts_url = "http://%s/cgi-bin/nagios3/config.cgi?"; \
> -                    "type=contacts" % unit.u.public_address
> +    contancts_url = (
> +        "http://%s/cgi-bin/nagios3/config.cgi?"; "type=contacts" % unit.u.public_address

remove " "

> +    )
>      contact_name = set_extra_contacts
>      r = requests.get(contancts_url, auth=auth)
>      assert r.status_code == 200, "Get Nagios config request failed"
>      assert contact_name in r.text, "Nagios is not loading the extra contact."
> -    assert contact_name.capitalize() in r.text, "Contact name alias is not " \
> -                                                "the capitalized name."
> -    contactgroups_url = "http://%s/cgi-bin/nagios3/config.cgi"; \
> -                        "?type=contactgroups" % unit.u.public_address
> +    assert contact_name.capitalize() in r.text, (
> +        "Contact name alias is not " "the capitalized name."

remove " "

> +    )
> +    contactgroups_url = (
> +        "http://%s/cgi-bin/nagios3/config.cgi";
> +        "?type=contactgroups" % unit.u.public_address
> +    )
>  
>      r = requests.get(contactgroups_url, auth=auth)
>      assert r.status_code == 200, "Get Nagios config request failed"
> -    assert contact_name in r.text, "Extra contact is not " \
> -                                   "added to the contact groups."
> +    assert contact_name in r.text, (
> +        "Extra contact is not " "added to the contact groups."

remove " "

> +    )
> +
>  
>  async def test_multiple_admin_contacts(auth, unit, set_multiple_admins):
> -    contancts_url = "http://%s/cgi-bin/nagios3/config.cgi?"; \
> -                    "type=contacts" % unit.u.public_address
> +    contancts_url = (
> +        "http://%s/cgi-bin/nagios3/config.cgi?"; "type=contacts" % unit.u.public_address

remove " "

> +    )
>      admins = set_multiple_admins
>      r = requests.get(contancts_url, auth=auth)
>      assert r.status_code == 200, "Get Nagios config request failed"
> -    admins = admins.split(',')
> +    admins = admins.split(",")
>      for admin in admins:
> -        admin = admin.replace('@', '').replace('.', '').lower()
> +        admin = admin.replace("@", "").replace(".", "").lower()
>          admin_alias = admin.capitalize()
>          assert admin in r.text, "Nagios is not loading contact {}.".format(admin)
> -        assert admin_alias in r.text, "Nagios is not loading alias " \
> -                                      "for contact {}.".format(admin)
> +        assert (
> +            admin_alias in r.text
> +        ), "Nagios is not loading alias " "for contact {}.".format(admin)

remove " "

> diff --git a/tests/functional/test_deploy.py b/tests/functional/test_deploy.py
> index b8e9f40..e4131d6 100644
> --- a/tests/functional/test_deploy.py
> +++ b/tests/functional/test_deploy.py
> @@ -22,17 +23,15 @@ async def test_web_interface_is_protected(auth, unit):
>  
>  
>  async def test_hosts_being_monitored(auth, unit):
> -    host_url = ("http://%s/cgi-bin/nagios3/status.cgi?";
> -                "hostgroup=all&style=hostdetail") % unit.u.public_address
> +    host_url = (
> +        "http://%s/cgi-bin/nagios3/status.cgi?"; "hostgroup=all&style=hostdetail"

remove " "

> +    ) % unit.u.public_address
>      r = requests.get(host_url, auth=auth)
> -    assert 'mysql' in r.text, "Nagios is not monitoring the hosts it supposed to."
> +    assert "mysql" in r.text, "Nagios is not monitoring the hosts it supposed to."
>  
>  
>  async def test_nrpe_monitors_config(relatives, unit, file_contents):
>      # look for disk root check in nrpe config
> -    mysql_unit = relatives['mysql']['app'].units[0]
> -    contents = await file_contents(
> -            '/etc/nagios/nrpe.d/check_disk_root.cfg',
> -            mysql_unit
> -        )
> +    mysql_unit = relatives["mysql"]["app"].units[0]
> +    contents = await file_contents("/etc/nagios/nrpe.d/check_disk_root.cfg", mysql_unit)
>      assert contents, "disk root check config not found."


-- 
https://code.launchpad.net/~xavpaice/charm-nagios/+git/nagios-charm/+merge/388626
Your team Nagios Charm developers is subscribed to branch charm-nagios:master.