Le Minh Nghia has posted comments on this change. ( http://gerrit.cloudera.org:8080/10992 )
Change subject: IMPALA-6490: Reconnect shell when remote restarts ...................................................................... Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/10992/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10992/2//COMMIT_MSG@12 PS2, Line 12: now > now Done http://gerrit.cloudera.org:8080/#/c/10992/2/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/10992/2/shell/impala_client.py@33 PS2, Line 33: from thrift.Thrift import TApplicationException, TException : > nit: One line Done http://gerrit.cloudera.org:8080/#/c/10992/2/shell/impala_client.py@234 PS2, Line 234: ll be catched and return False.""" : if self.connect > Comment is still incorrect. Hopefully done. http://gerrit.cloudera.org:8080/#/c/10992/2/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/10992/2/shell/impala_shell.py@566 PS2, Line 566: nnected(): > test_connected() could be renamed to is_connected() or something similar to Done http://gerrit.cloudera.org:8080/#/c/10992/2/shell/impala_shell.py@566 PS2, Line 566: if not self.imp_client.is_connected(): : print_to_stderr > Introducing 'connected' is unnecessary. Mathod should be called in the if s Done http://gerrit.cloudera.org:8080/#/c/10992/2/tests/custom_cluster/test_shell_interactive_reconnect.py File tests/custom_cluster/test_shell_interactive_reconnect.py: http://gerrit.cloudera.org:8080/#/c/10992/2/tests/custom_cluster/test_shell_interactive_reconnect.py@84 PS2, Line 84: # reconnect. > Would it be possible to use ImpalaShell() object here instead of pexpect? ( I added comments which explain my decision http://gerrit.cloudera.org:8080/#/c/10992/2/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/10992/2/tests/shell/test_shell_interactive.py@43 PS2, Line 43: SHELL_HISTORY_FILE = os.path.expanduser("~/.impalahistory") > 'START_CLUSTER' is not used anywhere, you can remove this line. Done -- To view, visit http://gerrit.cloudera.org:8080/10992 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia13365a9696886f01294e98054cf4e7cd66ab712 Gerrit-Change-Number: 10992 Gerrit-PatchSet: 3 Gerrit-Owner: Le Minh Nghia <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Le Minh Nghia <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 20 Jul 2018 12:34:53 +0000 Gerrit-HasComments: Yes
