Change in vdsm[master]: jobs: Fix abort semantics

[Nir Soffer]

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: 


-- 
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 <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes