Code review comment for lp:~azzar1/update-manager/canonical-livepatch

Revision history for this message
Andrea Azzarone (azzar1) wrote :

That's a problem with the specification. With livepatch you either get all
the patches correctly applied or none.

Il 26 ago 2017 00:29, "Brian Murray" <email address hidden> ha scritto:

> Review: Needs Fixing
>
> How will these changes allow a message like "7 Livepatch updates applied
> since the last restart. 1 other update failed to apply.”, from the
> specification, to be displayed? Does set_desc allow both the failure and
> success messages to be displayed?
>
> Diff comments:
>
> >
> > === modified file 'UpdateManager/Dialogs.py'
> > --- UpdateManager/Dialogs.py 2017-08-07 20:54:58 +0000
> > +++ UpdateManager/Dialogs.py 2017-08-23 14:09:35 +0000
> > @@ -142,6 +144,45 @@
> > self.main_container.add(content_widget)
> > self.main_container.set_visible(bool(content_widget))
> >
> > + def on_livepatch_status_ready(self, active, check_state,
> patch_state, fixes):
> > + self.set_desc(None)
> > +
> > + if not active:
> > + return
> > +
> > + needs_reschedule = False
> > +
> > + if check_state == "needs-check":
> > + needs_reschedule = True
> > + elif check_state == "check-failed":
> > + pass
> > + elif check_state == "checked":
> > + if patch_state == "unapplied" or patch_state == "applying":
> > + needs_reschedule = True
> > + elif patch_state == "applied":
> > + patched_fixes = [fix for fix in fixes if fix.patched]
> > + if len(patched_fixes) == 1:
> > + self.set_desc(_("1 Livepatch update applied since
> the last restart."))
> > + elif len(patched_fixes) > 1:
> > + self.set_desc(_("%d Livepatch updates applied since
> the last restart." % len(patched_fixes)))
> > + elif patch_state == "applied-with-bug" or patch_state ==
> "apply-failed":
> > + patched_fixes = [fix for fix in fixes if fix.patched]
> > + if len(patched_fixes) == 1:
> > + self.set_desc(_("1 update failed to apply since
> the last restart"))
>
> The other calls to set_desc end with a period so lets have this one also
> end with a period.
>
> > + elif len(patched_fixes) > 1:
> > + self.set_desc(_("%d updates failed to apply since
> the last restart" % len(patched_fixes)))
>
> The other calls to set_desc end with a period so lets have this one also
> end with a period.
>
> > + elif patch_state == "nothing-to-apply":
> > + pass
> > + elif patch_state == "unknown":
> > + pass
> > +
> > + if needs_reschedule:
> > + self.lp_socket.get_status(self.on_livepatch_status_ready)
> > +
> > + def check_livepatch_status(self):
> > + self.lp_socket = LivePatchSocket()
> > + self.lp_socket.get_status(self.on_livepatch_status_ready)
> > +
> >
> > class StoppedUpdatesDialog(InternalDialog):
> > def __init__(self, window_main):
>
>
> --
> https://code.launchpad.net/~azzar1/update-manager/
> canonical-livepatch/+merge/329452
> You are the owner of lp:~azzar1/update-manager/canonical-livepatch.
>
> Launchpad-Message-Rationale: Owner
> Launchpad-Message-For: azzar1
> Launchpad-Notification-Type: code-review
> Launchpad-Branch: ~azzar1/update-manager/canonical-livepatch
> Launchpad-Project: update-manager
>

« Back to merge proposal