Nir Soffer has posted comments on this change.
Change subject: jobs: Fix abort semantics ......................................................................
Patch Set 4: Code-Review+1
(5 comments)
https://gerrit.ovirt.org/#/c/65102/4/lib/vdsm/jobs.py File lib/vdsm/jobs.py:
Line 146 Line 147 Line 148 Line 149 Line 150 We can log here that we are starting the job.
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:
Aborted Job xxxyyy Line 137: self._status = STATUS.ABORTED Line 138: if self.autodelete: Line 139: self._autodelete() Line 140: elif self.status == STATUS.RUNNING:
Line 148: raise JobNotActive() Line 149: Line 150: def run(self): Line 151: if not self._may_run(): Line 152: return We need log info for starting the job - maybe inside _may_run? Line 153: try: Line 154: self._run() Line 155: except exception.ActionStopped: Line 156: self._abort_completed()
Line 155: except exception.ActionStopped: Line 156: self._abort_completed() Line 157: except Exception as e: Line 158: self._run_failed(e) Line 159: else: For extra consistency, we can introduce _run_completed() for these lines. Line 160: with self._status_lock: Line 161: self._status = STATUS.DONE Line 162: finally: Line 163: if self.autodelete:
Line 157: except Exception as e: Line 158: self._run_failed(e) Line 159: else: Line 160: with self._status_lock: Line 161: self._status = STATUS.DONE What we miss in the successful case ia log info about job completing. Line 162: finally: Line 163: if self.autodelete: Line 164: self._autodelete() Line 165: