Nir Soffer has posted comments on this change. Change subject: Update v2v to use new jobs infrastructure ......................................................................
Patch Set 1: Code-Review-1 (11 comments) https://gerrit.ovirt.org/#/c/45382/1/vdsm/v2v.py File vdsm/v2v.py: Line 97 Line 98 Line 99 Line 100 Line 101 We don't need this, just use jobs.JobExistsError Line 101 Line 102 Line 103 Line 104 Line 105 This error needs a name - looks like a bug in the current code. ClientError should have consistent interface. I would fix this in a separate patch. Line 106 Line 107 Line 108 Line 109 Line 110 We don't need this, just use jobs.NoSuchJob Line 111 Line 112 Line 113 Line 114 Line 115 Use jobs.JobNotDone Line 116 Line 117 Line 118 Line 119 Line 120 Should rename err_name to name to match jobs.ClientError. Line 124 Line 125 Line 126 Line 127 Line 128 Needs an error name. Line 202 Line 203 Line 204 Line 205 Line 206 ClientError does not have err_name now, use e.name Line 259: super(ImportVm, self).__init__(job_id) Line 260: self._vminfo = vminfo Line 261: self._irs = irs Line 262: Line 263: self._status = V2VSTATUS.STARTING We can make the patch smaller by keeping v2v.STATUS (it can still inherit from jobs.STATUS) Line 264: self._disk_progress = 0 Line 265: self._disk_count = 1 Line 266: self._current_disk = 1 Line 267: self._aborted = False Line 325: if self._aborted: Line 326: logging.debug("Job %r was aborted", self._id) Line 327: else: Line 328: logging.exception("Job %r failed", self._id) Line 329: self._status = V2VSTATUS.FAILED Can be avoided Line 330: self._description = ex.message Line 331: try: Line 332: self._abort() Line 333: except Exception as e: Line 355: (self._id, self._proc.returncode, Line 356: self._proc.stderr.read(1024))) Line 357: Line 358: if self._status != V2VSTATUS.ABORTED: Line 359: self._status = V2VSTATUS.DONE Can be avoided Line 360: logging.info('Job %r finished import successfully', self._id) Line 361: Line 362: def _execution_environments(self): Line 363: env = {'LIBGUESTFS_BACKEND': 'direct'} Line 376: def _watch_process_output(self): Line 377: parser = OutputParser() Line 378: for event in parser.parse(self._proc.stdout): Line 379: if isinstance(event, ImportProgress): Line 380: self._status = V2VSTATUS.COPYING_DISK Can be avoided Line 381: logging.info("Job %r copying disk %d/%d", Line 382: self._id, event.current_disk, event.disk_count) Line 383: self._disk_progress = 0 Line 384: self._current_disk = event.current_disk -- To view, visit https://gerrit.ovirt.org/45382 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9118e0fe4aaeceab2109afa393dd45fbd97e070f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
