impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zoltan Borok-Nagy (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-2235: Fix current db when shell auto-reconnects
Date Thu, 02 Nov 2017 19:52:28 GMT
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 <boroknagyz@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <philip@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <boroknagyz@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Nov 2017 19:52:28 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message