Adam Litke has posted comments on this change. Change subject: Update v2v to use new jobs infrastructure ......................................................................
Patch Set 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 Done Line 101 Line 102 Line 103 Line 104 Line 105 > This error needs a name - looks like a bug in the current code. ClientError Agreed, this is a general v2v bug and not part of this refactoring. Line 106 Line 107 Line 108 Line 109 Line 110 > We don't need this, just use jobs.NoSuchJob Done Line 111 Line 112 Line 113 Line 114 Line 115 > Use jobs.JobNotDone Done Line 116 Line 117 Line 118 Line 119 Line 120 > Should rename err_name to name to match jobs.ClientError. V2VErrors are not descendant from jobs.ClientError so they needn't conform to it's interface. They probably should, but I think that would be an unrelated patch. Line 124 Line 125 Line 126 Line 127 Line 128 > Needs an error name. Agreed, but not related to this patch. If this can be raised as an API level error we'd also need a new errCode defined to match it. Line 202 Line 203 Line 204 Line 205 Line 206 > ClientError does not have err_name now, use e.name Done 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 f Done 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 Done 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 Done 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 Done 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
