[Merge] lp:~azzar1/software-properties/add-canonical-livepatch into lp:software-properties

Didier Roche didrocks at ubuntu.com
Tue Mar 20 09:07:36 UTC 2018


Good work! I'm having some remarks in addition to Brian's comment, please see them inline.

My major concern is error handling, which is using strings (empty strings or none) to tell that no error happened instead of using python exceptions. In some cases (see my inline reviews), it means that you even don't make difference between legit "no error executing this command" and "error without any output of the command", which is problematic IMHO.

The rest is majoritaly some syntax suggestions to make the code more readable and pythonish, not all required, but some changes may be beneficials IMHO.

Also:
* You should refresh the translation template file in po/software-properties.pot and check that every added strings is indeed in it.
* Now that seb opened a FFe bug, refer to it in debian/changelog

Note that I didn't test it yet, will do once the changes are committed here.

Diff comments:

> 
> === added file 'softwareproperties/GoaAuth.py'
> --- softwareproperties/GoaAuth.py	1970-01-01 00:00:00 +0000
> +++ softwareproperties/GoaAuth.py	2018-03-16 13:02:28 +0000
> @@ -0,0 +1,113 @@
> +#
> +#  Copyright (c) 2018 Canonical
> +#
> +#  Authors:
> +#       Andrea Azzarone <andrea.azzarone at canonical.com>
> +#
> +#  This program is free software; you can redistribute it and/or
> +#  modify it under the terms of the GNU General Public License as
> +#  published by the Free Software Foundation; either version 2 of the
> +#  License, or (at your option) any later version.
> +#
> +#  This program is distributed in the hope that it will be useful,
> +#  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +#  GNU General Public License for more details.
> +#
> +#  You should have received a copy of the GNU General Public License
> +#  along with this program; if not, write to the Free Software
> +#  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> +#  USA
> +
> +import gi
> +gi.require_version('Goa', '1.0')
> +gi.require_version('Secret', '1')
> +from gi.repository import Goa, GObject, Secret
> +
> +SECRETS_SCHEMA = Secret.Schema.new('com.ubuntu.SotwareProperties',
> +                                   Secret.SchemaFlags.NONE,
> +                                   {'key': Secret.SchemaAttributeType.STRING})
> +
> +class GoaAuth(GObject.GObject):
> +
> +    # Properties
> +    logged = GObject.Property(type=bool, default=False)
> +
> +    def __init__(self):
> +        GObject.GObject.__init__(self)
> +
> +        self.goa_client = Goa.Client.new_sync(None)
> +        self.account = None
> +        self._load()
> +
> +    def login(self, account):
> +        assert(account)
> +        self._update_state(account)
> +        self._store()
> +
> +    def logout(self):
> +        self._update_state(None)
> +        self._store()
> +
> +    @GObject.Property
> +    def username(self):
> +        if self.account is None:
> +            return None
> +        return self.account.props.presentation_identity
> +
> +    @GObject.Property
> +    def token(self):
> +        if self.account is None:
> +            return None
> +
> +        obj = self.goa_client.lookup_by_id(self.account.props.id)
> +        if obj is None:
> +            return None
> +
> +        pbased = obj.get_password_based()
> +        if pbased is None:
> +            return None
> +
> +        return pbased.call_get_password_sync('livepatch')
> +
> +    def _update_state_from_account_id(self, account_id):
> +        if account_id:
> +            # Make sure the account-id is valid
> +            obj = self.goa_client.lookup_by_id(account_id)
> +            if obj is None:
> +                self._update_state(None)
> +                return

Shouldn't that raise an exception rather? To differentiate initiale state "None" and exception retrieved? (I put that comment here, but it's in general overall the code)

> +
> +            account = obj.get_account()
> +            if account is None:
> +                self._update_state(None)
> +                return
> +
> +            self._update_state(account)
> +        else:
> +            self._update_state(None)
> +
> +    def _update_state(self, account):
> +        self.account = account
> +        if self.account is None:
> +            self.logged = False
> +        else:
> +            try:
> +                account.call_ensure_credentials_sync(None)
> +            except Exception:
> +                self.logged = False
> +            else:
> +                self.account.connect('notify::attention-needed', lambda o, v: self.logout())
> +                self.logged = True
> +
> +    def _load(self):
> +        # Retrieve the stored account-id
> +        account_id = Secret.password_lookup_sync(SECRETS_SCHEMA, {'key': 'account-id'}, None)
> +        self._update_state_from_account_id(account_id)
> +
> +    def _store(self):
> +        if self.logged:
> +            account_id = self.account.props.id
> +            Secret.password_store(SECRETS_SCHEMA, {'key': 'account-id'}, None, 'com.ubuntu.SoftwareProperties', account_id)
> +        else:
> +            Secret.password_clear(SECRETS_SCHEMA, {'key': 'account-id'}, None, None, None)
> 
> === modified file 'softwareproperties/SoftwareProperties.py'
> --- softwareproperties/SoftwareProperties.py	2017-03-01 17:35:37 +0000
> +++ softwareproperties/SoftwareProperties.py	2018-03-16 13:02:28 +0000
> @@ -858,6 +866,128 @@
>      except:
>        return False
>  
> +  #
> +  # Livepatch
> +  #
> +  def init_snapd(self):
> +      self.snapd_client = Snapd.Client()
> +
> +  def get_livepatch_snap_async(self, callback):
> +      assert self.snapd_client
> +      self.snapd_client.list_one_async('canonical-livepatch',
> +                                        self.cancellable,
> +                                        self.on_list_one_ready_cb,
> +                                        callback)
> +
> +  def on_list_one_ready_cb(self, source_object, result, user_data):
> +      callback = user_data
> +      try:
> +          snap = source_object.list_one_finish(result)
> +      except:
> +          snap = None
> +      if snap:
> +          if callback: callback(snap)
> +          return
> +      else:
> +          assert self.snapd_client
> +          self.snapd_client.find_async(Snapd.FindFlags.MATCH_NAME,
> +                                       'canonical-livepatch',
> +                                        self.cancellable,
> +                                        self.on_find_ready_cb,
> +                                        callback)
> +
> +  def on_find_ready_cb(self, source_object, result, user_data):
> +      callback = user_data
> +      try:
> +          snaps = source_object.find_finish(result)[0]
> +      except:
> +          snaps = list()
> +      snap = snaps[0] if len(snaps) else None
> +      if callback: callback(snap)

I don't really like ternary expression, but that's ok if you like it :)

However, on the if callback: do that, this is different frmo the rest of the file syntax, I would do it in 2 lines.

> +
> +  def get_livepatch_snap_status(self, snap):
> +      if snap is None:
> +          return Snapd.SnapStatus.UNKNOWN
> +      return snap.get_status()
> +
> +  # glib-snapd does not keep track of the status of the snap. Use this decorator
> +  # to make it easy to write async functions that will always have an updated
> +  # snap object.
> +  def require_livepatch_snap(func):

Prefer using style-class decorator if it's the first one of the project (I didn't check if there is others): http://python-3-patterns-idioms-test.readthedocs.io/en/latest/PythonDecorators.html

> +      def get_livepatch_snap_and_call(*args, **kwargs):
> +          return args[0].get_livepatch_snap_async(lambda snap: func(snap=snap, *args, **kwargs))
> +      return get_livepatch_snap_and_call
> +
> +  def is_livepatch_enabled(self):
> +    file = Gio.File.new_for_path(path='/var/snap/canonical-livepatch/common/machine-token')

For tests, I would pass that as a parameter (and have a default constant on top of the file). That's a IMHO :)

> +    return file.query_exists(None)
> +
> +  @require_livepatch_snap
> +  def set_livepatch_enabled_async(self, enabled, token, callback, snap=None):
> +      status = self.get_livepatch_snap_status(snap)
> +      if status == Snapd.SnapStatus.UNKNOWN:
> +          if callback: callback(True, _("canonical-livepatch cannot be installed"))

Same callback (and below, not repeating it) remark

> +      elif status == Snapd.SnapStatus.ACTIVE:
> +          if enabled:
> +              error = self.enable_livepatch_service(token)
> +          else:
> +              error = self.disable_livepatch_service()
> +          if callback: callback(len(error) > 0, error)
> +      elif status == Snapd.SnapStatus.INSTALLED:
> +          if enabled:
> +              self.snapd_client.enable_async(name='canonical-livepatch',
> +                                             cancellable=self.cancellable,
> +                                             callback=self.livepatch_enable_snap_cb,
> +                                             user_data=(callback, token))
> +          else:
> +              if callback: callback(False, "")
> +      elif status == Snapd.SnapStatus.AVAILABLE:
> +          if enabled:
> +              self.snapd_client.install_async(name='canonical-livepatch',
> +                                              channel='edge', # Remove this once bionic is officialy supported.
> +                                              cancellable=self.cancellable,
> +                                              callback=self.livepatch_install_snap_cb,
> +                                              user_data=(callback, token))
> +          else:
> +              if callback: callback(False, "")
> +
> +  def livepatch_enable_snap_cb(self, source_object, result, user_data): 
> +      (callback, token) = user_data
> +      try:
> +          if source_object.enable_finish(result):
> +              error = self.enable_livepatch_service(token)
> +              if callback: callback(len(error) > 0, error)
> +      except Exception:
> +          if callback: callback(True, _("canonical-livepatch cannot be installed"))
> +
> +  def livepatch_install_snap_cb(self, source_object, result, user_data):
> +      (callback, token) = user_data
> +      try:
> +          if source_object.install_finish(result):
> +              error = self.enable_livepatch_service(token)
> +              if callback: callback(len(error) > 0, error)
> +      except Exception:
> +          if callback: callback(True, _("canonical-livepatch cannot be installed"))
> +
> +  def enable_livepatch_service(self, token):
> +    if self.is_livepatch_enabled():
> +      return ""
> +
> +    try:
> +      subprocess.check_output(['/snap/bin/canonical-livepatch', 'enable', token], stderr=subprocess.STDOUT)
> +      return ""
> +    except subprocess.CalledProcessError as e:
> +      return e.output if e.output else ""

It seems that you treat here (and below) error as strings, and lose the exception context.

For instance, here, you don't differentiate:
- successful canonical-livepatch call
- a subprocess.CalledProcessError exception, when the binary doesn't print anything on stdout or stderr but exit with !=0.

> +
> +  def disable_livepatch_service(self):
> +    if not self.is_livepatch_enabled():
> +      return ""
> +
> +    try:
> +      subprocess.check_output(['/snap/bin/canonical-livepatch', 'disable'], stderr=subprocess.STDOUT)
> +      return ""
> +    except subprocess.CalledProcessError as e:
> +      return e.output if e.output else ""

Same than above

>  
>  def shortcut_handler(shortcut):
>      for factory in _SHORTCUT_FACTORIES:
> 
> === added file 'softwareproperties/gtk/DialogAuth.py'
> --- softwareproperties/gtk/DialogAuth.py	1970-01-01 00:00:00 +0000
> +++ softwareproperties/gtk/DialogAuth.py	2018-03-16 13:02:28 +0000
> @@ -0,0 +1,218 @@
> +#
> +#  Copyright (c) 2018 Canonical
> +#
> +#  Authors:
> +#       Andrea Azzarone <andrea.azzarone at canonical.com>
> +#
> +#  This program is free software; you can redistribute it and/or
> +#  modify it under the terms of the GNU General Public License as
> +#  published by the Free Software Foundation; either version 2 of the
> +#  License, or (at your option) any later version.
> +#
> +#  This program is distributed in the hope that it will be useful,
> +#  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +#  GNU General Public License for more details.
> +#
> +#  You should have received a copy of the GNU General Public License
> +#  along with this program; if not, write to the Free Software
> +#  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> +#  USA
> +
> +import os
> +
> +from gettext import gettext as _
> +from softwareproperties.gtk.utils import (
> +    setup_ui,
> +)
> +
> +import gi
> +gi.require_version('Goa', '1.0')
> +from gi.repository import Gio, GLib, Goa, GObject, Gtk
> +
> +
> +class DialogAuth:
> +
> +    def __init__(self, parent, datadir):
> +        """setup up the gtk dialog"""
> +        self.parent = parent
> +
> +        setup_ui(self, os.path.join(datadir, "gtkbuilder",
> +                                    "dialog-auth.ui"), domain="software-properties")
> +        self.label_title.set_max_width_chars(40)
> +
> +        self.dialog = self.dialog_auth
> +        self.dialog.use_header_bar = True
> +        self.dialog.set_transient_for(parent)
> +
> +        self.account = None
> +        self.dispose_on_new_account = False
> +        self.goa_client = Goa.Client.new_sync(None)
> +
> +        self.button_settings.connect('clicked', self._button_settings_clicked_cb)
> +        self.listbox_accounts.connect('row-activated', self._listbox_accounts_row_activated_cb)
> +
> +        # Be ready to other accounts
> +        self.goa_client.connect('account-added', self._account_added_cb)
> +        self.goa_client.connect('account-removed', self._account_removed_cb)
> +
> +        self._setup_listbox_accounts()
> +        self._check_ui()
> +
> +    def run(self):
> +        res = self.dialog.run()
> +        self.dialog.hide()
> +        return res
> +
> +    def _check_ui(self):
> +        rows = self.listbox_accounts.get_children()
> +        has_accounts = len(rows) > 1
> +
> +        if has_accounts:
> +            self.label_title.set_text(_('To continue choose an Ubuntu Single Sign-On account.'))
> +            self.label_new_account.set_markup('<b>{}</b>'.format(_('Use another account…')))

<3 you use "…" char! /me doesn't feel lonely anymore :)

> +        else:
> +            self.label_title.set_text(_('To continue you need an Ubuntu Single Sign-On account.'))
> +            self.label_new_account.set_markup('<b>{}</b>'.format(_('Sign in…')))

<3 <3

However, here I would personnally do it like this, to not repeat widget assignement:

if has_accounts:
   title = _(…)
   new_account = _(…)
else:
   title = _(…)
   new_account = _(…)

self.label_title.set_text(title)
self.label_new_account.set_markup('<b>{}</b>'.format(new_account))

> +
> +    def _setup_listbox_accounts(self):
> +        for obj in self.goa_client.get_accounts():
> +            account = obj.get_account()
> +            if self._is_account_supported(account):
> +                self._add_account(account)
> +
> +    def _is_account_supported(self, account):
> +        return account.props.provider_type == 'ubuntusso'
> +
> +    def _add_account(self, account):
> +        row = self._create_row(account)
> +        self.listbox_accounts.prepend(row)
> +        self._check_ui()
> +
> +    def _remove_account(self, account):
> +        for row in self.listbox_accounts.get_children():
> +            if hasattr(row, 'account') and row.account == account:

I would rather:

try:
   if row.account == account:
      row.destroy()
      self._check_ui()
      break
except …:
   <some debug logging>

> +                row.destroy()
> +                self._check_ui()
> +                break
> +
> +    def _build_dbus_params(self, action, arg):
> +        builder = GLib.VariantBuilder.new(GLib.VariantType.new('av'))
> +
> +        if action is None and arg is None:
> +            s = GLib.Variant.new_string('')
> +            v = GLib.Variant.new_variant(s)
> +            builder.add_value(v)
> +        else:
> +            if action is not None:
> +                s = GLib.Variant.new_string(action)
> +                v = GLib.Variant.new_variant(s)
> +                builder.add_value(v)
> +            if arg is not None:
> +                s = GLib.Variant.new_string(arg)
> +                v = GLib.Variant.new_variant(s)
> +                builder.add_value(v)
> +
> +        array = GLib.Variant.new_tuple(GLib.Variant.new_string('online-accounts'), builder.end())
> +        array = GLib.Variant.new_variant(array)
> +
> +        param = GLib.Variant.new_tuple(
> +            GLib.Variant.new_string('launch-panel'),
> +            GLib.Variant.new_array(GLib.VariantType.new('v'), [array]),
> +            GLib.Variant.new_array(GLib.VariantType.new('{sv}'), None))
> +        return param
> +
> +    def _spawn_goa_with_args(self, action, arg):
> +        proxy = Gio.DBusProxy.new_for_bus_sync(Gio.BusType.SESSION,
> +            Gio.DBusProxyFlags.NONE, None,
> +            'org.gnome.ControlCenter',
> +            '/org/gnome/ControlCenter',
> +            'org.gtk.Actions', None)
> +
> +        param = self._build_dbus_params(action, arg)
> +        proxy.call_sync('Activate', param, Gio.DBusCallFlags.NONE, -1, None)
> +
> +    def _create_row(self, account):
> +        identity = account.props.presentation_identity
> +        provider_name = account.props.provider_name
> +
> +        row = Gtk.ListBoxRow.new()
> +        row.show()
> +
> +        hbox = Gtk.Box.new(Gtk.Orientation.HORIZONTAL, 0)
> +        hbox.set_hexpand(True)
> +        hbox.show()
> +
> +        image = Gtk.Image.new_from_icon_name('avatar-default', Gtk.IconSize.DIALOG)
> +        image.set_pixel_size(48)
> +        image.show()
> +        hbox.pack_start(image, False, False, 0)
> +
> +        vbox = Gtk.Box.new(Gtk.Orientation.VERTICAL, 0)
> +        vbox.show()
> +        hbox.pack_start(vbox, False, False, 0)
> +
> +        if identity:
> +            ilabel = Gtk.Label.new()
> +            ilabel.set_halign(Gtk.Align.START)
> +            ilabel.set_markup('<b>{}</b>'.format(identity))
> +            ilabel.show()
> +            vbox.pack_start(ilabel, True, True, 0)
> +
> +        if provider_name:
> +            plabel = Gtk.Label.new()
> +            plabel.set_halign(Gtk.Align.START)
> +            plabel.set_markup('<small><span foreground=\"#555555\">{}</span></small>'.format(provider_name))
> +            plabel.show()
> +            vbox.pack_start(plabel, True, True, 0)
> +
> +        warning_icon = Gtk.Image.new_from_icon_name('dialog-warning-symbolic', Gtk.IconSize.BUTTON)
> +        warning_icon.set_no_show_all(True)
> +        warning_icon.set_margin_end (15)
> +        hbox.pack_end(warning_icon, False, False, 0)
> +
> +        row.add(hbox)
> +
> +        account.bind_property('attention-needed', warning_icon, 'visible',
> +            GObject.BindingFlags.DEFAULT | GObject.BindingFlags.SYNC_CREATE)
> +
> +        row.account = account
> +        return row
> +
> +    # Signals handlers
> +    def _button_settings_clicked_cb(self, button):
> +        self._spawn_goa_with_args(None, None)
> +
> +    def _listbox_accounts_row_activated_cb(self, listbox, row):
> +        account = row.account if hasattr(row, 'account') else None
> +
> +        if account is None:
> +            # TODO (azzar1): there is no easy way to put this to false
> +            # if the user close the windows without adding an account.
> +            # We need to discuss with goa's upstream to support such usercases
> +            self.dispose_on_new_account = True
> +            self._spawn_goa_with_args('add', 'ubuntusso')
> +        else:
> +            if account.props.attention_needed:
> +                self._spawn_goa_with_args(account.props.id, None)
> +            else:
> +                self.account = account
> +                self.dialog.response(Gtk.ResponseType.OK)
> +
> +    def _account_added_cb(self, goa_client, goa_object):
> +        account = goa_object.get_account()
> +        if not self._is_account_supported(account):
> +            return
> +        if not self.dispose_on_new_account:
> +            self._add_account(account)
> +        else:
> +            self.account = account
> +            self.dialog.response(Gtk.ResponseType.OK)
> +
> +    def _account_removed_cb(self, goa_client, goa_object):
> +        account = goa_object.get_account()
> +        if self._is_account_supported(account):
> +            self._remove_account(account)
> +
> +
> +
> 
> === modified file 'softwareproperties/gtk/SoftwarePropertiesGtk.py'
> --- softwareproperties/gtk/SoftwarePropertiesGtk.py	2018-03-06 15:47:27 +0000
> +++ softwareproperties/gtk/SoftwarePropertiesGtk.py	2018-03-16 13:02:28 +0000
> @@ -79,6 +83,8 @@
>  ) = list(range(5))
>  
>  
> +INFINIT_DBUS_TIMEOUT = 0x7fffffff/1000

Is that really safe? What happens if goa daemon hangs forever?

> +
>  def error(parent_window, summary, msg):
>      """ show a error dialog """
>      dialog = Gtk.MessageDialog(parent=parent_window,
> @@ -1446,3 +1459,138 @@
>                                                 % {'count': self.nonfree_drivers})
>          else:
>              self.label_driver_action.set_label(_("No proprietary drivers are in use."))
> +
> +    #
> +    # Livepatch
> +    #
> +    def init_livepatch(self):
> +        self.goa_auth = GoaAuth()
> +        self.waiting_livepatch_response = False
> +        self.quit_when_livepatch_responds = False
> +
> +        if not self.is_livepatch_supported():
> +            self.grid_livepatch.set_visible(False)
> +            return
> +
> +        self.checkbutton_livepatch.set_active(self.is_livepatch_enabled())
> +        self.on_goa_auth_changed()
> +
> +        # hacky way to monitor if livepatch is enabled or not
> +        file = Gio.File.new_for_path(path='/var/snap/canonical-livepatch/common/machine-token')

Reuse the same const than in the other file, to have that info in one place.

> +        self.lp_monitor = file.monitor_file(Gio.FileMonitorFlags.NONE)
> +
> +        # connect to signals
> +        self.handlers[self.goa_auth] = \
> +            self.goa_auth.connect('notify::logged', lambda o, p: self.on_goa_auth_changed())
> +        self.handlers[self.checkbutton_livepatch] = \
> +            self.checkbutton_livepatch.connect('toggled', self.on_checkbutton_livepatch_toggled)
> +        self.handlers[self.button_ubuntuone] =  \
> +            self.button_ubuntuone.connect('clicked', self.on_button_ubuntuone_clicked)
> +        self.handlers[self.lp_monitor] = \
> +            self.lp_monitor.connect('changed', self.on_livepatch_status_changed)
> +
> +    def is_livepatch_supported(self):
> +        supported_releases = ['trusty', 'xenial', 'bionic']
> +        return any([self.distro.is_codename(d) for d in supported_releases])
> +
> +    def on_goa_auth_changed(self):
> +        if self.goa_auth.logged:
> +            self.button_ubuntuone.set_label(_('Sign Out'))
> +
> +            if self.goa_auth.token:
> +                self.checkbutton_livepatch.set_sensitive(True)
> +                self.label_livepatch_login.set_label(_('Signed in as %s' % self.goa_auth.username))
> +            else:
> +                self.checkbutton_livepatch.set_sensitive(False)
> +                text = _('%s isn\'t authorized to use Livepatch.' % self.goa_auth.username)
> +                text = "<span color='red'>" +  text + "</span>"
> +                self.label_livepatch_login.set_markup(text) 
> +        else:
> +            self.checkbutton_livepatch.set_sensitive(False)
> +            self.button_ubuntuone.set_label(_('Sign In…'))
> +            self.label_livepatch_login.set_label(_('To use Livepatch you need to sign in.'))
> +
> +    def on_livepatch_status_changed(self, file_monitor, file, other_file, event_type):
> +        if not self.waiting_livepatch_response:
> +            self.checkbutton_livepatch.set_active(self.is_livepatch_enabled())
> +
> +    def on_button_ubuntuone_clicked(self, button):
> +        if self.goa_auth.logged:
> +            self.do_logout()
> +        else:
> +            self.do_login()
> +
> +    def do_login(self):
> +        try:
> +            # Show login dialog!
> +            dialog = DialogAuth(self.window_main, self.datadir)
> +            response = dialog.run()
> +        except Exception as e:
> +            logging.error(e)
> +            error(self.window_main,
> +                  _("Error enabling Canonical Livepatch"),
> +                  _("Please check your Internet connection."))
> +        else:
> +            if response == Gtk.ResponseType.OK:
> +                self.goa_auth.login(dialog.account)
> +                if self.goa_auth.logged:
> +                    self.checkbutton_livepatch.set_active(True)
> +
> +    def do_logout(self):
> +        self.goa_auth.logout()
> +        self.checkbutton_livepatch.set_active(False)
> +
> +    def on_checkbutton_livepatch_toggled(self, checkbutton):
> +        if self.waiting_livepatch_response:
> +            return
> +
> +        self.waiting_livepatch_response = True
> +
> +        if self.checkbutton_livepatch.get_active():
> +            self.backend.SetLivepatchEnabled(True, self.goa_auth.token if self.goa_auth.token else '',
> +                                             reply_handler=self.livepatch_enabled_reply_handler,
> +                                             error_handler=self.livepatch_enabled_error_handler,
> +                                             timeout=INFINIT_DBUS_TIMEOUT)
> +        else:
> +            self.backend.SetLivepatchEnabled(False, '',
> +                                             reply_handler=self.livepatch_enabled_reply_handler,
> +                                             error_handler=self.livepatch_enabled_error_handler,
> +                                             timeout=INFINIT_DBUS_TIMEOUT)
> +
> +    def livepatch_enabled_reply_handler(self, is_error, prompt):
> +        self.sync_checkbutton_livepatch(is_error, prompt)
> +
> +    def livepatch_enabled_error_handler(self, e):
> +        if e._dbus_error_name == 'com.ubuntu.SoftwareProperties.PermissionDeniedByPolicy':
> +            logging.error("Authentication canceled, changes have not been saved")
> +            self.sync_checkbutton_livepatch(is_error=True, prompt=None)
> +        else:
> +            self.sync_checkbutton_livepatch(is_error=True, prompt=str(e))
> +
> +    def sync_checkbutton_livepatch(self, is_error, prompt):
> +        if is_error:
> +            self.waiting_livepatch_response = False
> +            self.checkbutton_livepatch.handler_block(self.handlers[self.checkbutton_livepatch])
> +            self.checkbutton_livepatch.set_active(self.is_livepatch_enabled())
> +            self.checkbutton_livepatch.handler_unblock(self.handlers[self.checkbutton_livepatch])
> +
> +            if prompt:
> +                dialog = DialogLivepatchError(self.window_main, self.datadir)
> +                response = dialog.run(prompt)
> +                if response == DialogLivepatchError.RESPONSE_SETTINGS:
> +                    self.window_main.show()
> +                    self.quit_when_livepatch_responds = False
> +        else:
> +            if self.is_livepatch_enabled() and not self.checkbutton_livepatch.get_active():
> +                self.backend.SetLivepatchEnabled(False, '',
> +                                                 reply_handler=self.livepatch_enabled_reply_handler,
> +                                                 error_handler=self.livepatch_enabled_error_handler)
> +            elif not self.is_livepatch_enabled() and self.checkbutton_livepatch.get_active():
> +                self.backend.SetLivepatchEnabled(True, self.goa_auth.token if self.goa_auth.token else '',
> +                                                 reply_handler=self.livepatch_enabled_reply_handler,
> +                                                 error_handler=self.livepatch_enabled_error_handler)
> +            else:
> +                self.waiting_livepatch_response = False
> +
> +        if self.quit_when_livepatch_responds:
> +            self.on_close_button(self.button_close)


-- 
https://code.launchpad.net/~azzar1/software-properties/add-canonical-livepatch/+merge/341427
Your team Ubuntu Core Development Team is requested to review the proposed merge of lp:~azzar1/software-properties/add-canonical-livepatch into lp:software-properties.



More information about the Ubuntu-reviews mailing list