Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8368 )
Change subject: IMPALA-2235: Fix current db when shell auto-reconnects ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/8368/2/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/8368/2/shell/impala_shell.py@729 PS2, Line 729: def _validate_database(self, immediately=False): > Is there a case where you ever want immediately=True? i.e., could we get ri Yes, in precmd after reconnecting to the cluster (this commit). Every other _validate_database invocation use the default value which maintains the old behaviour of the method. http://gerrit.cloudera.org:8080/#/c/8368/2/tests/common/cluster_controller.py File tests/common/cluster_controller.py: http://gerrit.cloudera.org:8080/#/c/8368/2/tests/common/cluster_controller.py@34 PS2, Line 34: breakpad > Comment needs update. I guess now it's used for all custom cluster tests th Yeah, that's the idea. I'll remove the comment. http://gerrit.cloudera.org:8080/#/c/8368/2/tests/common/cluster_controller.py@42 PS2, Line 42: self.tmp_dir = tempfile.mkdtemp() > I'm thinking about whether this is factored in the best way and whether the Seems reasonable, will do it http://gerrit.cloudera.org:8080/#/c/8368/2/tests/custom_cluster/test_shell_interactive_reconnect.py File tests/custom_cluster/test_shell_interactive_reconnect.py: http://gerrit.cloudera.org:8080/#/c/8368/2/tests/custom_cluster/test_shell_interactive_reconnect.py@27 PS2, Line 27: class TestShellInteractiveReconnect(ClusterController): > Any reason not to add this simply to tests/shell/test_shell_interactive.py? Yes, it is in the custom cluster test suite because I am starting/restarting the cluster. http://gerrit.cloudera.org:8080/#/c/8368/2/tests/custom_cluster/test_shell_interactive_reconnect.py@40 PS2, Line 40: @pytest.mark.execute_serially > I think all CustomCluster tests run serially? This one seems like it could I guess you are right, my idea was what if another test case restarts the cluster during this test, so this impala shell automatically reconnects to the cluster and it makes this test fail or pass. What do you think? -- To view, visit http://gerrit.cloudera.org:8080/8368 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I40dfa00ba0314d356fe8617446f516505c925e5e Gerrit-Change-Number: 8368 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Thu, 02 Nov 2017 19:52:28 +0000 Gerrit-HasComments: Yes
