Tomas Golembiovsky has posted comments on this change. Change subject: compat: py3: capture output of CPopen.communicate ......................................................................
Patch Set 2: (2 comments) https://gerrit.ovirt.org/#/c/61393/2//COMMIT_MSG Commit Message: PS2, Line 12: stdin you mean stdout/stderr https://gerrit.ovirt.org/#/c/61393/2/lib/vdsm/compat.py File lib/vdsm/compat.py: Line 51: if 'stderr' not in kwargs: Line 52: kwargs['stderr'] = subprocess.PIPE Line 53: if 'stdout' not in kwargs: Line 54: kwargs['stdout'] = subprocess.PIPE Line 55: subprocess.Popen.__init__(self, args, **kwargs) > please set explicitly stderr and stdout to PIPE in execCmd - that way only I aggree. This should be fixed in execCmd. And the few cases that use CPopen directly should be reviewed and fixed if necessary. Otherwise we'll just end up reimplementing old CPopen behaviour over subprocess.Popen. Line 56: Line 57: try: Line 58: from contextlib import suppress Line 59: suppress # make pyflakes happy -- To view, visit https://gerrit.ovirt.org/61393 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iac9c1c1bd3ca1902103fac1f8675b7d42ca78a33 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Irit Goihman <igoih...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Tomas Golembiovsky <tgole...@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/admin/lists/vdsm-patches@lists.fedorahosted.org
