Dan Kenigsberg has posted comments on this change. Change subject: v2v: Convert VM from external source to Data Domain ......................................................................
Patch Set 7: (3 comments) https://gerrit.ovirt.org/#/c/37509/7/vdsm/v2v.py File vdsm/v2v.py: Line 196: self._vmProperties['vmName']) Line 197: Line 198: logging.info('import vm, (jobId %s) started, cmd: %r', self._job_id, Line 199: cmd) Line 200: proc = subprocess.Popen(cmd, env={'LIBGUESTFS_BACKEND': 'direct'}, > I will change the name (it was handling input and output before we use the you should use sync=False if you want to poll stdout line by line. Line 201: shell=True, Line 202: stdout=subprocess.PIPE, Line 203: stderr=subprocess.PIPE) Line 204: Line 214: logging.info('import vm ended, exit status %d', proc.returncode) Line 215: if proc.returncode != 0: Line 216: self._status = 'ended with error: %r' % proc.returncode Line 217: else: Line 218: self._status = 'done' please define 'done' and other states as a CONSTANT. Line 219: Line 220: def _handle_process_input(self, proc): Line 221: re_copy_disk = re.compile(r'.*(Copying disk \d+/\d+).*') Line 222: while not self._abort: Line 219: Line 220: def _handle_process_input(self, proc): Line 221: re_copy_disk = re.compile(r'.*(Copying disk \d+/\d+).*') Line 222: while not self._abort: Line 223: line = proc.stdout.readline() > sure the test will be challenging... This is my point exactly - parsing output should be separated from the flow management. I'd like to see simple and testable functions with simple textual input and programmatic output. Then, they can be assembled into the complete flow. Line 224: Line 225: if line == '': Line 226: self._handle_process_errors(proc) Line 227: break -- To view, visit https://gerrit.ovirt.org/37509 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I34bd86d5a87ea8c42113c4a732f87ddd4ceab9ea Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
