Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/10992 )
Change subject: IMPALA-6490: Reconnect shell when remote restarts ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/10992/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10992/1//COMMIT_MSG@9 PS1, Line 9: If the remote impalad died while the shell waited for nit: can you wrap lines in git commits at 60 chars? formatting in git tools prefer this If you edit commit messages with git you can have it auto set this w/ this in your vimrc: au FileType gitcommit set tw=60 http://gerrit.cloudera.org:8080/#/c/10992/1/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/10992/1/shell/impala_client.py@233 PS1, Line 233: """Checks to see if the current Impala connection is still alive. If not, an exception : will be raised.""" Comment is out of date now. http://gerrit.cloudera.org:8080/#/c/10992/1/shell/impala_client.py@235 PS1, Line 235: if self.connected: : return self.imp_service.PingImpalaService() Since test_connection() now returns a True/False value, maybe we should catch exceptions here (and return False) instead of requiring the caller to deal with it. Also, it would make the code cleaner if we returned False explicitly if self.conneced is False. http://gerrit.cloudera.org:8080/#/c/10992/1/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/10992/1/tests/shell/test_shell_interactive.py@157 PS1, Line 157: @pytest.mark.execute_serially : def test_auto_reconnect(self): : """Test reconnect after restarting the remote impalad without using connect;""" : kill_cluster = " ".join([START_CLUSTER, "--kill"]) : pexpect.run(kill_cluster) : pexpect.run(START_CLUSTER) : proc = pexpect.spawn(SHELL_CMD) : proc.expect(":21000] default>") : # Disconnect : pexpect.run(kill_cluster) : proc.sendline("show tables;") : proc.expect("[Not connected]") : # Restarting Impalad : pexpect.run(START_CLUSTER) : # Check reconnect : proc.sendline("show tables;") : proc.expect("large_table") Please check ./tests/custom_cluster/test_shell_interactive_reconnect.py. Probably this test should be moved there. -- 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: 1 Gerrit-Owner: Le Minh Nghia <le.ng...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Thu, 19 Jul 2018 15:23:56 +0000 Gerrit-HasComments: Yes
