Change in vdsm[master]: compat: py3: capture output of CPopen.communicate
Dan Kenigsberg has submitted this change and it was merged. Change subject: compat: py3: capture output of CPopen.communicate .. compat: py3: capture output of CPopen.communicate To allow better coverage of python3 testing, we use Popen instead of CPopen, as the latter is not implemented (and not needed) in python3. Unfortunately, CPopen has chosen to use stdout\err=subprocess.PIPE as its default. Now when cpopen supports stdout and strerr params we can always set those to PIPE to match the defaults. Change-Id: Iac9c1c1bd3ca1902103fac1f8675b7d42ca78a33 Signed-off-by: Dan Kenigsberg Signed-off-by: Yaniv Bronhaim Reviewed-on: https://gerrit.ovirt.org/61393 Continuous-Integration: Jenkins CI Reviewed-by: Piotr Kliczewski Reviewed-by: Irit Goihman --- M lib/vdsm/commands.py 1 file changed, 3 insertions(+), 0 deletions(-) Approvals: Piotr Kliczewski: Looks good to me, but someone else must approve Yaniv Bronhaim: Verified Jenkins CI: Passed CI tests Irit Goihman: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved -- To view, visit https://gerrit.ovirt.org/61393 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iac9c1c1bd3ca1902103fac1f8675b7d42ca78a33 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: compat: py3: capture output of CPopen.communicate
Dan Kenigsberg has posted comments on this change. Change subject: compat: py3: capture output of CPopen.communicate .. Patch Set 4: Code-Review+2 -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: compat: py3: capture output of CPopen.communicate
gerrit-hooks has posted comments on this change. Change subject: compat: py3: capture output of CPopen.communicate .. Patch Set 5: * Update tracker: IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: compat: py3: capture output of CPopen.communicate
Irit Goihman has posted comments on this change. Change subject: compat: py3: capture output of CPopen.communicate .. Patch Set 4: Code-Review+1 -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: compat: py3: capture output of CPopen.communicate
Piotr Kliczewski has posted comments on this change. Change subject: compat: py3: capture output of CPopen.communicate .. Patch Set 4: Code-Review+1 -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: compat: py3: capture output of CPopen.communicate
Yaniv Bronhaim has posted comments on this change. Change subject: compat: py3: capture output of CPopen.communicate .. Patch Set 4: Verified+1 -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: compat: py3: capture output of CPopen.communicate
gerrit-hooks has posted comments on this change. Change subject: compat: py3: capture output of CPopen.communicate .. Patch Set 4: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: compat: py3: capture output of CPopen.communicate
Tomas Golembiovsky has posted comments on this change. Change subject: compat: py3: capture output of CPopen.communicate .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/61393/3/lib/vdsm/commands.py File lib/vdsm/commands.py: Line 67: Line 68: execCmdLogger.debug(cmdutils.command_log_line(printable, cwd=cwd)) Line 69: Line 70: extra = {} Line 71: extra['stderr'] = subprocess.PIPE You're missing import subprocess. Line 72: extra['stdout'] = subprocess.PIPE Line 73: if deathSignal is not None: Line 74: extra['deathSignal'] = deathSignal Line 75: p = CPopen(command, close_fds=True, cwd=cwd, env=env, **extra) -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: compat: py3: capture output of CPopen.communicate
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 Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: compat: py3: capture output of CPopen.communicate
Yaniv Bronhaim has posted comments on this change. Change subject: compat: py3: capture output of CPopen.communicate .. Patch Set 2: (1 comment) 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 direct usages in CPopen which we don't have, will need this compat logic 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 Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: compat: py3: capture output of CPopen.communicate
gerrit-hooks has posted comments on this change. Change subject: compat: py3: capture output of CPopen.communicate .. Patch Set 2: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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 Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: compat: py3: capture output of CPopen.communicate
Dan Kenigsberg has uploaded a new change for review. Change subject: compat: py3: capture output of CPopen.communicate .. compat: py3: capture output of CPopen.communicate To allow better coverage of python3 testing, we use Popen instead of CPopen, as the latter is not implemented (and not needed) in python3. Unfortunately, CPopen has chosen to use stdin=subprocess.PIPE as its default. vdsm.compat.CPopen must follow suit, or the process's output would not be returned by CPopen.communicate. Change-Id: Iac9c1c1bd3ca1902103fac1f8675b7d42ca78a33 Signed-off-by: Dan Kenigsberg --- M lib/vdsm/compat.py 1 file changed, 8 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/61393/1 diff --git a/lib/vdsm/compat.py b/lib/vdsm/compat.py index 8884005..414c193 100644 --- a/lib/vdsm/compat.py +++ b/lib/vdsm/compat.py @@ -44,8 +44,14 @@ from cpopen import CPopen CPopen # make pyflakes happy else: -from subprocess import Popen as CPopen -CPopen # make pyflakes happy +import subprocess +class CPopen(subprocess.Popen): +def __init__(self, args, **kwargs): +if 'stderr' not in kwargs: +kwargs['stderr'] = subprocess.PIPE +if 'stdout' not in kwargs: +kwargs['stdout'] = subprocess.PIPE +subprocess.Popen.__init__(self, args, **kwargs) try: from contextlib import suppress -- To view, visit https://gerrit.ovirt.org/61393 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iac9c1c1bd3ca1902103fac1f8675b7d42ca78a33 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: compat: py3: capture output of CPopen.communicate
gerrit-hooks has posted comments on this change. Change subject: compat: py3: capture output of CPopen.communicate .. Patch Set 1: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org