Le Minh Nghia has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )
Change subject: IMPALA-1624: Allow toggling some command-line options inside impala-shell ...................................................................... Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py@135 PS1, Line 135: alaShell.TRUE_STRINGS, "pri > This could be moved to a separate variable like TRUE_STRINGS or the whole l Done http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py@137 PS1, Line 137: pal > Will this handle other special characters like tabs and new lines? Yes, it will. http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py@138 PS1, Line 138: RUE_ > I would prefer an empty string instead on 'None' to be consistent with the Done http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py@1435 PS1, Line 1435: > I have noticed that this tip is wrong. Please correct the default to \t. It Done http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py@98 PS1, Line 98: assert "21,VIETNAM,2" in result.stdout > Please also add a positive test (a row that should be included in the resul Done http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py@119 PS1, Line 119: assert "VIETNAM" not in result.stdout > It should be also verified that the results were written to the file. Done http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py@125 PS1, Line 125: p2 = ImpalaShell() > The primary way to set back options to their default should be calling "uns Done -- To view, visit http://gerrit.cloudera.org:8080/10900 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763 Gerrit-Change-Number: 10900 Gerrit-PatchSet: 2 Gerrit-Owner: Le Minh Nghia <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Jinchul Kim <[email protected]> Gerrit-Reviewer: Le Minh Nghia <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 13 Jul 2018 15:20:18 +0000 Gerrit-HasComments: Yes
