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?