Francesco Romani has posted comments on this change. Change subject: jobs: Fix abort semantics ......................................................................
Patch Set 4: (2 comments) initial review. Looks nice, but I need to doublecheck the state machine. https://gerrit.ovirt.org/#/c/65102/4/lib/vdsm/jobs.py File lib/vdsm/jobs.py: Line 132: if self.status == STATUS.PENDING: Line 133: # Autodelete should only be handled here for pending state. Line 134: # There is no operation running so we can go straight to Line 135: # aborted state. In all other cases, autodelete is handled as Line 136: # the _run method finishes. > We need to log here, something like: +1 (for consistency purposes from my POV) logging.info() should be fine. Line 137: self._status = STATUS.ABORTED Line 138: if self.autodelete: Line 139: self._autodelete() Line 140: elif self.status == STATUS.RUNNING: PS4, Line 177: _abort_completed perhaps _run_aborted(self) could be a bit clearer? -- To view, visit https://gerrit.ovirt.org/65102 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I801082c50b10cf0571210d65cd3a5cec0d282a5c Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
