Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15524 )
Change subject: IMPALA-3343, IMPALA-9489: Make impala-shell compatible with python 3. ...................................................................... Patch Set 4: (5 comments) I did a pass over it. I'm not sure that I full understood all the details, so might want to do another pass later on, but I think relying on the end-to-end tests to validate this makes sense. One thing was that I couldn't get the built tarball to work with python 3 on my system http://gerrit.cloudera.org:8080/#/c/15524/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15524/4//COMMIT_MSG@121 PS4, Line 121: processes. I'm running into an issue where if I run the packaged shell with python3, sqlparse barfs: tarmstrong@tarmstrong-box2:~/Impala/impala$ ./shell/build/impala-shell-3.4.0-SNAPSHOT/impala-shell -B --output_delimiter='??' Traceback (most recent call last): File "/home/tarmstrong/Impala/impala/shell/build/impala-shell-3.4.0-SNAPSHOT/impala_shell.py", line 35, in <module> import sqlparse File "<frozen importlib._bootstrap>", line 971, in _find_and_load File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked File "<frozen importlib._bootstrap>", line 656, in _load_unlocked File "<frozen importlib._bootstrap>", line 626, in _load_backward_compatible File "/home/tarmstrong/Impala/impala/shell/build/impala-shell-3.4.0-SNAPSHOT/ext-py/sqlparse-0.1.19-py2.7.egg/sqlparse/__init__.py", line 13, in <module> File "<frozen importlib._bootstrap>", line 971, in _find_and_load File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked File "<frozen importlib._bootstrap>", line 656, in _load_unlocked File "<frozen importlib._bootstrap>", line 626, in _load_backward_compatible File "/home/tarmstrong/Impala/impala/shell/build/impala-shell-3.4.0-SNAPSHOT/ext-py/sqlparse-0.1.19-py2.7.egg/sqlparse/engine/__init__.py", line 8, in <module> File "<frozen importlib._bootstrap>", line 971, in _find_and_load File "<frozen importlib._bootstrap>", line 951, in _find_and_load_unlocked File "<frozen importlib._bootstrap>", line 894, in _find_spec File "<frozen importlib._bootstrap_external>", line 1157, in find_spec File "<frozen importlib._bootstrap_external>", line 1131, in _get_spec File "<frozen importlib._bootstrap_external>", line 1112, in _legacy_get_spec File "<frozen importlib._bootstrap>", line 441, in spec_from_loader File "<frozen importlib._bootstrap_external>", line 544, in spec_from_file_location File "/home/tarmstrong/Impala/impala/shell/build/impala-shell-3.4.0-SNAPSHOT/ext-py/sqlparse-0.1.19-py2.7.egg/sqlparse/lexer.py", line 84 except Exception, err: ^ I guess the shell tarball only supports python 2 at the moment and the pip-installed package can run with newer versions of the dependency? http://gerrit.cloudera.org:8080/#/c/15524/4/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/15524/4/shell/impala_shell.py@1633 PS4, Line 1633: if not isinstance(input_string, str): nit: could express as a single if with an "and" clause instead of nested ifs OK to ignore http://gerrit.cloudera.org:8080/#/c/15524/4/shell/impala_shell.py@1790 PS4, Line 1790: if isinstance(options.output_delimiter, str): This seems to barf if I pass in a unicode delimiter. I don't think this is important but wanted to pass along $ ./shell/build/impala-shell-3.4.0-SNAPSHOT/impala-shell -B --output_delimiter='??' /home/tarmstrong/Impala/impala/shell/build/impala-shell-3.4.0-SNAPSHOT/lib/option_parser.py:325: UnicodeWarning: Unicode equal comparison failed to convert both arguments to Unicode - interpreting them as being unequal if '--live_progress' in sys.argv and '--disable_live_progress' in sys.argv: /home/tarmstrong/Impala/impala/shell/build/impala-shell-3.4.0-SNAPSHOT/lib/option_parser.py:328: UnicodeWarning: Unicode equal comparison failed to convert both arguments to Unicode - interpreting them as being unequal if '--verbose' in sys.argv and '--quiet' in sys.argv: Traceback (most recent call last): File "/home/tarmstrong/Impala/impala/shell/build/impala-shell-3.4.0-SNAPSHOT/impala_shell.py", line 1940, in <module> impala_shell_main() File "/home/tarmstrong/Impala/impala/shell/build/impala-shell-3.4.0-SNAPSHOT/impala_shell.py", line 1791, in impala_shell_main delim_sequence = bytearray(options.output_delimiter, 'utf-8') UnicodeDecodeError: 'ascii' codec can't decode byte 0xf0 in position 0: ordinal not in range(128) http://gerrit.cloudera.org:8080/#/c/15524/4/shell/make_shell_tarball.sh File shell/make_shell_tarball.sh: http://gerrit.cloudera.org:8080/#/c/15524/4/shell/make_shell_tarball.sh@52 PS4, Line 52: if [ "${USE_THRIFT11_GEN_PY:-}" == "false" ]; then Does it make sense to allow overriding this? Would a shell built with old thrift even work? http://gerrit.cloudera.org:8080/#/c/15524/4/shell/make_shell_tarball.sh@53 PS4, Line 53: # thrift 0.9.3-p7 Not sure if we need this comment? -- To view, visit http://gerrit.cloudera.org:8080/15524 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb004d352fe230a890a6b6356496ba76c2fab615 Gerrit-Change-Number: 15524 Gerrit-PatchSet: 4 Gerrit-Owner: David Knupp <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 25 Mar 2020 01:17:28 +0000 Gerrit-HasComments: Yes
