nagios-charmers team mailing list archive

[Joel Sing <joel.sing@xxxxxxxxxxxxx>]

Review: Approve +1

LGTM, but see comment inline.

Diff comments:

> diff --git a/src/tests/functional/conftest.py b/src/tests/functional/conftest.py
> index ea22194..cbc7d9f 100644
> --- a/src/tests/functional/conftest.py
> +++ b/src/tests/functional/conftest.py
> @@ -137,7 +137,15 @@ async def file_stat(run_command):  # pylint: disable=redefined-outer-name
>      async def _file_stat(path, target):
>          cmd = STAT_CMD % path
>          results = await run_command(cmd, target)
> -        return json.loads(results['Stdout'])
> +        if results['Code'] == '0':

Nit: This is fine, or you could do the following which may be slightly more readable:

if results['Code'] != '0':
  ...
  raise

return .loads(results['Stdout'])

> +            return json.loads(results['Stdout'])
> +        else:
> +            # A common possible error is simply ENOENT, the file ain't there.
> +            # A better solution would be to retrieve the exception that the
> +            # remote python code raised, but that would probably require a real
> +            # RPC setup
> +            raise RuntimeError('Stat failed: {}'.format(results))
> +
>      return _file_stat
>  
>  


-- 
https://code.launchpad.net/~aieri/hw-health-charm/+git/hw-health-charm/+merge/364759
Your team Nagios Charm developers is subscribed to branch hw-health-charm:oo-rewrite-integration.