livepatch-charmers team mailing list archive

[Paul Gear <paul.gear@xxxxxxxxxxxxx>]

Review: Needs Fixing

See Stuart's comments about errors.  I think all but one need changing.

Diff comments:

> diff --git a/reactive/canonical_livepatch.py b/reactive/canonical_livepatch.py
> index bb9d547..fbdaf01 100644
> --- a/reactive/canonical_livepatch.py
> +++ b/reactive/canonical_livepatch.py
> @@ -214,10 +217,20 @@ def activate_livepatch():
>  def livepatch_supported():
>      arch = uname()[4]
>      opts = layer.options('snap')
> -    supported_archs = opts['canonical-livepatch'].pop('supported-architectures', None)
> +    supported_archs = opts['canonical-livepatch'].pop(
> +        'supported-architectures',
> +        None
> +    )
>      if supported_archs and arch not in supported_archs:
> -        hookenv.log('Livepatch does not currently support this architecture: {}'.format(arch))
> -        unit_update('blocked', 'Architecture {} is not supported by livepatch'.format(arch))
> +        hookenv.log(
> +            'Livepatch does not currently support {} architecture'.format(
> +                arch
> +            )
> +        )

I don't think that this one should be an error.  We want the charm to deploy without error and sit in blocked state if livepatch is not applicable to that architecture.

> +        unit_update(
> +            'blocked',
> +            'Architecture {} is not supported by livepatch'.format(arch)
> +        )
>      if is_container():
>          hookenv.log('OS container detected, livepatch is not needed')
>          unit_update('blocked', 'Livepatch is not needed in OS containers')


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