Sahil Takiar 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 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/15524/6/bin/impala-shell.sh File bin/impala-shell.sh: http://gerrit.cloudera.org:8080/#/c/15524/6/bin/impala-shell.sh@27 PS6, Line 27: USE_THRIFT11_GEN_PY who sets this? the user? http://gerrit.cloudera.org:8080/#/c/15524/6/shell/packaging/requirements.txt File shell/packaging/requirements.txt: http://gerrit.cloudera.org:8080/#/c/15524/6/shell/packaging/requirements.txt@8 PS6, Line 8: thrift==0.11.0 a bit confused about the version of Thrift being used. after this patch, will impala-shell always use thrift 11, regardless of whether it runs on Python 2 or 3? some of the changes in bin/impala-shell.sh seems to indicate there is a way to switch between using the thrift 11 vs thrift 9 code? http://gerrit.cloudera.org:8080/#/c/15524/6/shell/shell_output.py File shell/shell_output.py: http://gerrit.cloudera.org:8080/#/c/15524/6/shell/shell_output.py@101 PS6, Line 101: with open(self.filename, 'ab') as out_file: this seems to change the logic so that it opens and closed the file during each call to write, whereas the previous logic would just open the file once and then close it when the object is deleted. what's the reason for changing the logic? http://gerrit.cloudera.org:8080/#/c/15524/6/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/15524/6/tests/shell/test_shell_commandline.py@472 PS6, Line 472: # args = ['-q', "select '{0}'".format(RUSSIAN_CHARS)] delete? -- 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: 6 Gerrit-Owner: David Knupp <dkn...@cloudera.com> Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: David Knupp <dkn...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Mon, 06 Apr 2020 17:50:51 +0000 Gerrit-HasComments: Yes
