Change in vdsm[master]: schema: introduce HostJobInfo type

fromani at redhat.com fromani at redhat.com
Tue Dec 15 07:39:53 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: schema: introduce HostJobInfo type
......................................................................


Patch Set 2:

(6 comments)

overall seems OK, suggested possible improvements, a few minor questions inside

https://gerrit.ovirt.org/#/c/49450/2//COMMIT_MSG
Commit Message:

Line 10: host. We support multiple job types (v2v, storage, etc.) and all job
Line 11: types will be reported via the same Host.getJobs API call. There will
Line 12: also be a standard interface for aborting and/or deleting them.
Line 13: 
Line 14: This patch adds new "pending" job status, used as the default state.
this could be a separate change
Line 15: This is needed for storage jobs which are executed in a thread pool.
Line 16: The jobs module and tests were updated to support the new status.
Line 17: 
Line 18: This schema specify job "progress" property as optional and add a new


Line 14: This patch adds new "pending" job status, used as the default state.
Line 15: This is needed for storage jobs which are executed in a thread pool.
Line 16: The jobs module and tests were updated to support the new status.
Line 17: 
Line 18: This schema specify job "progress" property as optional and add a new
same, this could be a separate change
Line 19: optional "error" property. These changes are implemented in the next
Line 20: patches.
Line 21: 
Line 22: Change-Id: Ibedc242e22de0f3c69e722596a03fb0538594009


https://gerrit.ovirt.org/#/c/49450/2/lib/api/vdsmapi-schema.json
File lib/api/vdsmapi-schema.json:

Line 4055: 
Line 4056: ##
Line 4057: # @ErrorInfo:
Line 4058: #
Line 4059: # Information about an error
Surprising, don't we have the error response already described elsewhere?
Line 4060: #
Line 4061: # @code:         An error code
Line 4062: #
Line 4063: # @message:      A free-form description of the error


Line 4074: # on a host.
Line 4075: #
Line 4076: # @id:           The job UUID
Line 4077: #
Line 4078: # @description:  A description of the job or the current status of the job
if this is for debug purposes, and meant not for client (Engine) consumption, maybe make it optional?
Line 4079: #                (for debugging purposes)
Line 4080: #
Line 4081: # @job_type:     The specific job type
Line 4082: #


https://gerrit.ovirt.org/#/c/49450/2/lib/vdsm/jobs.py
File lib/vdsm/jobs.py:

Line 26: _lock = threading.Lock()
Line 27: _jobs = {}
Line 28: 
Line 29: 
Line 30: class STATUS:
minor: I see you added "PENDING" - which is fine. The other fields looks reordered, could you explain why the reordering is needed? Is the new order of states the expected lifecycle of a Job?
Line 31:     '''
Line 32:     PENDING: Job is has not started yet
Line 33:     RUNNING: Job is running
Line 34:     DONE: Job has finished successfully


Line 112:         if self.status != STATUS.DONE:
Line 113:             raise JobNotDone("Job %r is %s" % (self.id, self.status))
Line 114: 
Line 115:     def validate_not_active(self):
Line 116:         if self.status not in (STATUS.DONE, STATUS.ABORTED, STATUS.FAILED):
again, not that important, but why this ordering?
Line 117:             raise JobNotDone("Job %r is %s" % (self.id, self.status))
Line 118: 
Line 119:     def _abort(self):
Line 120:         """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibedc242e22de0f3c69e722596a03fb0538594009
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list