You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Anonymous Coward (Code Review)" <ge...@cloudera.org> on 2016/11/20 02:02:12 UTC

[kudu-CR] KUDU-1706 Unable to build from source using Python 3

andy@phdata.io has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/5153

Change subject: KUDU-1706 Unable to build from source using Python 3
......................................................................

KUDU-1706 Unable to build from source using Python 3

Fixed python preflight and postflight scripts to support python version 2 and 3.Converted print statements to have parenthese print(). Replaced file() with preferred open(). Resolved some string concatentation issues. Added universal_newlines=True) to subprocess.Popen in shell function of postflight.py script because in python3 it would expect bytes instead of a string if not set. Tested on python 2.6.6, 2.7.12, 3.5.2

Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
---
M thirdparty/postflight.py
M thirdparty/preflight.py
2 files changed, 25 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/5153/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: andy@phdata.io

[kudu-CR] KUDU-1706 Unable to build from source using Python 3

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Jordan Birdsell has posted comments on this change.

Change subject: KUDU-1706 Unable to build from source using Python 3
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5153/3/thirdparty/preflight.py
File thirdparty/preflight.py:

PS3, Line 85: print(p.stdin, script)
> This is not equivalent functionality. The prior statement was "printing", o
yikes, meant to put ...`A method that works in python 2 and 3 is print(script, file=p.stdin)`...
Doesnt matter though, just go with the input param route.


-- 
To view, visit http://gerrit.cloudera.org:8080/5153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: andy@phdata.io
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1706 Unable to build from source using Python 3

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5153

to look at the new patch set (#3).

Change subject: KUDU-1706 Unable to build from source using Python 3
......................................................................

KUDU-1706 Unable to build from source using Python 3

Fixed python preflight and postflight scripts to support python version 2
and 3. Converted print statements to have parenthese print().
Replaced file() with preferred open(). Resolved some string concatentation
issues. Added universal_newlines=True) to subprocess.Popen in
shell function of postflight.py script because in python3 it would expect
bytes instead of a string if not set. Tested on python 2.6.6, 2.7.12, 3.5.2

Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
---
M thirdparty/postflight.py
M thirdparty/preflight.py
2 files changed, 25 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/5153/3
-- 
To view, visit http://gerrit.cloudera.org:8080/5153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: andy@phdata.io
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1706 Unable to build from source using Python 3

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
andy@phdata.io has uploaded a new patch set (#2).

Change subject: KUDU-1706 Unable to build from source using Python 3
......................................................................

KUDU-1706 Unable to build from source using Python 3

Fixed python preflight and postflight scripts to support python version 2
and 3. Converted print statements to have parenthese print().
Replaced file() with preferred open(). Resolved some string concatentation issues. Added universal_newlines=True) to subprocess.Popen in shell function of
postflight.py script because in python3 it would expect bytes instead of a
string if not set. Tested on python 2.6.6, 2.7.12, 3.5.2

Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
---
M thirdparty/postflight.py
M thirdparty/preflight.py
2 files changed, 25 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/5153/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: andy@phdata.io
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1706 Unable to build from source using Python 3

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Jordan Birdsell has posted comments on this change.

Change subject: KUDU-1706 Unable to build from source using Python 3
......................................................................


Patch Set 3:

(3 comments)

Thanks for the patch Andy, this will be pretty useful for those building on an OS that defaults to python 3 (ubuntu 16.04). Just a couple minor things

http://gerrit.cloudera.org:8080/#/c/5153/3/thirdparty/preflight.py
File thirdparty/preflight.py:

PS3, Line 60: print("***" + error_msg)
Nit: Not sure if it matters much, but for whats its worth, the comma in the current script delimits the two print params with a space whereas the + concatenates (no space). Maybe switch it back to a comma for consistency (or add a space)?


PS3, Line 81:  p = subprocess.Popen([CXX, '-o', '/dev/null', '-c', '-x', 'c++', '-'] + flags,
            :       stderr=subprocess.PIPE,
            :       stdout=subprocess.PIPE,
            :       stdin=subprocess.PIPE)
I suspect once you fix the issue below, you will also need to add the universal_newlines arg here as well.


PS3, Line 85: print(p.stdin, script)
This is not equivalent functionality. The prior statement was "printing", or writing, {script} to stdin for the process. A method that works in python 2 and 3 is print(script, p.stdin)..however, this requires the `future` library and I don't believe that that is standard on all distributions, so maybe its best to avoid it for this script. Best was to approach this is to remove this line altogether and pass script under the `input` parameter for p.communicate() on the line below (See https://docs.python.org/2/library/subprocess.html#subprocess.Popen.communicate). That is the best approach anyways.


-- 
To view, visit http://gerrit.cloudera.org:8080/5153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: andy@phdata.io
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1706 Unable to build from source using Python 3

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Jordan Birdsell has posted comments on this change.

Change subject: KUDU-1706 Unable to build from source using Python 3
......................................................................


Patch Set 5: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/5153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: andy@phdata.io
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1706 Unable to build from source using Python 3

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5153

to look at the new patch set (#4).

Change subject: KUDU-1706 Unable to build from source using Python 3
......................................................................

KUDU-1706 Unable to build from source using Python 3

Fixed python preflight and postflight scripts to support python
version 2 and 3. Converted print statements to have parenthese
print(). Replaced file() with preferred open(). Resolved some string
concatentation issues. Added universal_newlines=True) to
subprocess.Popen in shell function of postflight.py script because
in python3 it would expect bytes instead of a string if not set.
Tested on python 2.6.6, 2.7.12, 3.5.2

Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
---
M thirdparty/postflight.py
M thirdparty/preflight.py
2 files changed, 25 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/5153/4
-- 
To view, visit http://gerrit.cloudera.org:8080/5153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: andy@phdata.io
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1706 Unable to build from source using Python 3

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Jordan Birdsell has submitted this change and it was merged.

Change subject: KUDU-1706 Unable to build from source using Python 3
......................................................................


KUDU-1706 Unable to build from source using Python 3

Fixed python preflight and postflight scripts to support python
version 2 and 3. Converted print statements to have parenthese
print(). Replaced file() with preferred open(). Resolved some string
concatentation issues. Added universal_newlines=True) to
subprocess.Popen in shell function of postflight.py script because
in python3 it would expect bytes instead of a string if not set.
Tested on python 2.6.6, 2.7.12, 3.5.2

Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
Reviewed-on: http://gerrit.cloudera.org:8080/5153
Tested-by: Kudu Jenkins
Reviewed-by: Jordan Birdsell <jt...@apache.org>
---
M thirdparty/postflight.py
M thirdparty/preflight.py
2 files changed, 28 insertions(+), 26 deletions(-)

Approvals:
  Jordan Birdsell: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/5153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: andy@phdata.io
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1706 Unable to build from source using Python 3

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5153

to look at the new patch set (#5).

Change subject: KUDU-1706 Unable to build from source using Python 3
......................................................................

KUDU-1706 Unable to build from source using Python 3

Fixed python preflight and postflight scripts to support python
version 2 and 3. Converted print statements to have parenthese
print(). Replaced file() with preferred open(). Resolved some string
concatentation issues. Added universal_newlines=True) to
subprocess.Popen in shell function of postflight.py script because
in python3 it would expect bytes instead of a string if not set.
Tested on python 2.6.6, 2.7.12, 3.5.2

Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
---
M thirdparty/postflight.py
M thirdparty/preflight.py
2 files changed, 28 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/5153/5
-- 
To view, visit http://gerrit.cloudera.org:8080/5153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3fcc92cb185c3dc880e06b4d235c094c8b8cd4cf
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: andy@phdata.io
Gerrit-Reviewer: Jordan Birdsell <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins