Nir Soffer has posted comments on this change. Change subject: Introducing context manager to terminate async processes on internal fail ......................................................................
Patch Set 5: (2 comments) https://gerrit.ovirt.org/#/c/51407/5/lib/vdsm/commands.py File lib/vdsm/commands.py: Line 47: def terminating(proc): Line 48: try: Line 49: yield proc Line 50: finally: Line 51: if proc.returncode is None: > there is no proc.poll, and I don't want to access private parts. returncode poll is inherited from subprocess.Popen. returncode call poll only in AsyncProc. In Pyhton 2 and 3, it does not, and we want to be compatible with standard Popen api. See https://docs.python.org/2/library/subprocess.html#subprocess.Popen.poll Line 52: try: Line 53: proc.kill() Line 54: except Exception: Line 55: logging.exception("Cannot kill process %n" % proc.pid) Line 48: try: Line 49: yield proc Line 50: finally: Line 51: if proc.returncode is None: Line 52: try: > poll is sync and can't raise. what's the different? poll can raise OSError, but even if it could not raise we should write cleanup code as if everything can raise. Line 53: proc.kill() Line 54: except Exception: Line 55: logging.exception("Cannot kill process %n" % proc.pid) Line 56: if proc.returncode is None: -- To view, visit https://gerrit.ovirt.org/51407 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I003cce39d62dba5644937fafaf2c6e24c526c075 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
