Change in vdsm[master]: v2v: Job related actions

nsoffer at redhat.com nsoffer at redhat.com
Fri Apr 3 21:51:38 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: v2v: Job related actions
......................................................................


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/39361/3/vdsm/v2v.py
File vdsm/v2v.py:

Line 198:         job = _get_job(job_id)
Line 199:         job.abort()
Line 200:         _remove_job(job_id)
Line 201:     except ClientError as e:
Line 202:         logging.info('Job %r cannot abort job, error: %r', job_id, e)
Since all ClientError include now the job id, we don't have to log it here. This is true for all job verbs.
Line 203:         return errCode[e.err_name]
Line 204:     return {'status': doneCode}
Line 205: 
Line 206: 


Line 263: 
Line 264: def _get_job(job_id):
Line 265:     with _lock:
Line 266:         if job_id not in _jobs:
Line 267:             raise NoSuchJob()
> please make consistent with line 274 (If already asked, please explain why 
I asked to add the job id to all errors, so we can follow a job in the log even if some code did not log the job id, or the error escaped from this module to other code that does not know the job id.
Line 268:         return _jobs[job_id]
Line 269: 
Line 270: 
Line 271: def _remove_job(job_id):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a58583a88e2f17ce0847626cd601efbe1a00ac9
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek 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-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list