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.
--
Mailing list: https://launchpad.net/~livepatch-charmers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~livepatch-charmers
More help : https://help.launchpad.net/ListHelp
