Nir Soffer has posted comments on this change.

Change subject: Adding python3 run for nosetests
......................................................................


Patch Set 10:

(3 comments)

https://gerrit.ovirt.org/#/c/48051/10/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 630:     """
Line 631: 
Line 632:     # Moving import here so that import utils over python3 will work. 
Once
Line 633:     # python3-cpopen will be available this import should return to 
top
Line 634:     from cpopen import CPopen
We don't need cpopen on python 3.

Why not use the compat trick to get Popen from either cpopen or subprocess?
Line 635: 
Line 636:     command = cmdutils.wrap_command(command, with_ioclass=ioclass,
Line 637:                                     ioclassdata=ioclassdata, 
with_nice=nice,
Line 638:                                     with_setsid=setsid, 
with_sudo=sudo,


https://gerrit.ovirt.org/#/c/48051/10/tests/Makefile.am
File tests/Makefile.am:

Line 29
Line 30
Line 31
Line 32
Line 33
And add here _py2?

I guess we are going to live with both py3 and py2 for long time, so lets make 
it more clear.


Line 25:          devices \
Line 26:          integration \
Line 27:          $(NULL)
Line 28: 
Line 29: test_modules_python3 = \
Maybe _py3?
Line 30:        apiData.py \
Line 31:        cmdutilsTests.py \
Line 32:        concurrentTests.py \
Line 33:        $(NULL)


-- 
To view, visit https://gerrit.ovirt.org/48051
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83355cce2af9125e6f017017905056956cd17081
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to