Change in vdsm[master]: Refactor v2v jobs for reusability
alitke at redhat.com
alitke at redhat.com
Fri Aug 21 20:38:55 UTC 2015
Adam Litke has posted comments on this change.
Change subject: Refactor v2v jobs for reusability
......................................................................
Patch Set 3:
(7 comments)
https://gerrit.ovirt.org/#/c/44857/3//COMMIT_MSG
Commit Message:
Line 11: to manage long-running storage operations. In order to make this
Line 12: possible, factor out the job management parts of v2v.py into a new
Line 13: module jobs.py and create a generic Job class which contains the
Line 14: complete interface needed by the job manager. In addition, add some
Line 15: basic tests for Job objects and their management.
> please make sure to verify this patch using Engine.
I'm waiting for a time when the upstream engine and vdsm code actually work together :)
Line 16:
Line 17: Change-Id: Ida6b1c460c5030c820c540e836e423d4632410df
https://gerrit.ovirt.org/#/c/44857/3/vdsm/jobs.py
File vdsm/jobs.py:
Line 45: UNKNOWN: the job type is unknown
Line 46: V2V: the job is a v2v VM conversion job
Line 47: '''
Line 48: UNKNOWN = 'unknown'
Line 49: V2V = 'v2v'
> We can remove this class now.
ahh, yes... Done.
Line 50:
Line 51:
Line 52: class ClientError(Exception):
Line 53: ''' Base class for client error '''
Line 73: _JOB_TYPE = None
Line 74:
Line 75: def __init__(self, job_id, description=''):
Line 76: self._id = job_id
Line 77: self._status = STATUS.RUNNING
> We should check with v2v guys that engine is not expecting the previous val
Perhaps I should just add starting as a v2v-specific status and keep running in the generic job. Then they can continue using their terminology but the base class will be made more generic.
Line 78: self._description = description
Line 79:
Line 80: @property
Line 81: def id(self):
Line 115: May be implemented by child class
Line 116: """
Line 117:
Line 118:
Line 119: def clear():
> Lets add a comment that this should be called only by the tests. To make it
Done
Line 120: with _lock:
Line 121: _jobs.clear()
Line 122:
Line 123:
Line 120: with _lock:
Line 121: _jobs.clear()
Line 122:
Line 123:
Line 124: def delete_job(job_id):
> jobs.delete_job() looks redundant
Agreed. Will do.
Line 125: try:
Line 126: job = get_job(job_id)
Line 127: job.validate_not_running()
Line 128: _delete_job(job_id)
Line 150: ret[job_id] = {
Line 151: 'status': job.status,
Line 152: 'description': job.description,
Line 153: 'progress': job.progress,
Line 154: 'job_type': job.job_type,
> This is better done in Job.info():
Done
Line 155: }
Line 156: return ret
Line 157:
Line 158:
Line 177: del _jobs[job_id]
Line 178:
Line 179:
Line 180: # Automatically initialize on first import
Line 181: clear()
> This is not needed now, as clear() is not initializing anything. It is only
Done
--
To view, visit https://gerrit.ovirt.org/44857
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ida6b1c460c5030c820c540e836e423d4632410df
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Arik Hadas <ahadas at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list