livepatch-charmers team mailing list archive

[Stuart Bishop <stuart.bishop@xxxxxxxxxxxxx>]

Review: Approve

Looks good. Might want to look into layer:status for future work, per inline comments.

Diff comments:

> diff --git a/reactive/canonical_livepatch.py b/reactive/canonical_livepatch.py
> index 533e5a7..f9c1e6e 100644
> --- a/reactive/canonical_livepatch.py
> +++ b/reactive/canonical_livepatch.py
> @@ -57,6 +58,9 @@ def get_livepatch_status():
>      livepatch_status = ''
>      try:
>          livepatch_status = check_output(cmd, universal_newlines=True)
> +    except FileNotFoundError:
> +        # If the snap's not installed yet, don't crash out
> +        pass

It would be better if get_livepatch_status() never got called until after the snap was installed, perhaps by sprinkling around a few more @when('snap.installed.canonical-livepatch') guards.

If you include layer:status , you could do unit_update('waiting', 'Waiting for snap installation') here too, without worrying that it blocks a more important message such as the 'blocked' status. I've been using it a while and have started recommending it: https://github.com/juju-solutions/layer-status

>      except CalledProcessError:
>          # If the service hasn't been enabled yet, we'll get a process error - this is fine
>          pass


-- 
https://code.launchpad.net/~barryprice/canonical-livepatch-charm/+git/trusty-handlers/+merge/363795
Your team Livepatch charm developers is subscribed to branch canonical-livepatch-charm:master.