Adam Litke has posted comments on this change.

Change subject: jobs: Allow run and abort only from valid states
......................................................................


Patch Set 1:

(3 comments)

https://gerrit.ovirt.org/#/c/63711/1/lib/vdsm/jobs.py
File lib/vdsm/jobs.py:

Line 69:     ''' Only pending jobs may be run '''
Line 70:     name = 'CannotRunJob'
Line 71: 
Line 72: 
Line 73: class JobNotRunnable(ClientError):
> Leftover? I don't see any use.
oops.  Removing.
Line 74:     ''' Job cannot be run from its current state '''
Line 75:     name = 'JobNotRunnable'
Line 76: 
Line 77: 


Line 136:         return self.status in (STATUS.PENDING, STATUS.RUNNING)
Line 137: 
Line 138:     def abort(self):
Line 139:         if not self.active:
Line 140:             raise CannotAbortJob()
> CannotAbortJob means that job was in a state which can be aborted, but the 
Yeah agreed.  I was debating about going this route but your comments seal it.
Line 141:         self._status = STATUS.ABORTED
Line 142:         logging.info('Job %r aborting...', self._id)
Line 143:         self._abort()
Line 144:         if self.autodelete:


Line 145:             self._autodelete()
Line 146: 
Line 147:     def run(self):
Line 148:         if self.status != STATUS.PENDING:
Line 149:             raise CannotRunJob()
> We have two case here:
Done
Line 150:         self._status = STATUS.RUNNING
Line 151:         try:
Line 152:             self._run()
Line 153:         except Exception as e:


-- 
To view, visit https://gerrit.ovirt.org/63711
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8bc1ab264c5786b43fb409c3fab8913337778bf
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to