Change in vdsm[master]: compat: py3: capture output of CPopen.communicate

[danken]

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

[danken]

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

[automation]

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

[igoihman]

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]

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

[ybronhei]

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

[automation]

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]

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]

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

[ybronhei]

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

[automation]

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

[danken]

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

[automation]

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