netplan-developers team mailing list archive

[Martin Pitt <martin.pitt@xxxxxxxxxx>]

There's a bug and a style issue, comments inline.

Diff comments:

> diff --git a/src/netplan b/src/netplan
> index bdef802..df423e6 100755
> --- a/src/netplan
> +++ b/src/netplan
> @@ -67,11 +70,24 @@ def parse_args():
>      return args
>  
>  
> +def is_nm_snap_enabled():
> +    return subprocess.call(['systemctl', '--quiet', 'is-enabled', NM_SNAP_SERVICE_NAME], stderr=subprocess.DEVNULL) == 0
> +
> +
> +def nmcli(args):
> +    binary_name = 'nmcli'
> +
> +    if is_nm_snap_enabled():
> +        binary_name = 'network-manager.nmcli'
> +
> +    subprocess.call([binary_name]+args, stderr=subprocess.DEVNULL)

spaces around + please

> +
> +
>  def nm_running():  # pragma: nocover (covered in autopkgtest)
>      '''Check if NetworkManager is running'''
>  
>      try:
> -        subprocess.check_call(['nmcli', 'general'], stdout=subprocess.DEVNULL, stderr=subprocess.STDOUT)
> +        nmcli(['general'])

This breaks the actual check as nmcli() only does call(), not check_call(). I suggest to use check_call() in nmcli() and intercept CalledProcessError for the 'disconnect' case.

>          return True
>      except (OSError, subprocess.SubprocessError):
>          return False


-- 
https://code.launchpad.net/~morphis/netplan/+git/netplan/+merge/306607
Your team Developers of netplan is subscribed to branch netplan:master.