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 4: (3 comments) Thanks for the review. In addition I also added a time.sleep(1) to the test case test_auto_reconnect because I noticed that it wasn't stable. This sleep gives time for the ImpalaShell to successfully start and connect to the Impala daemon before we restart the cluster. http://gerrit.cloudera.org:8080/#/c/8368/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8368/3//COMMIT_MSG@9 PS3, Line 9: The ImpalaShell didn't issue the 'USE <current-db>' command after > This sentence didn't parse for me. Specifically, I'm not sure what "that" i I rephrased the beginning of the commit. I hope it is easier to understand now. http://gerrit.cloudera.org:8080/#/c/8368/3/tests/custom_cluster/test_shell_interactive_reconnect.py File tests/custom_cluster/test_shell_interactive_reconnect.py: http://gerrit.cloudera.org:8080/#/c/8368/3/tests/custom_cluster/test_shell_interactive_reconnect.py@36 PS3, Line 36: : cls.tempfile_name = tempfile.mktemp() : move_shell_history(cls.tempfile_n > Minor: I think this is just: I only used NamedTemporaryFile because mktemp is marked as deprecated: https://docs.python.org/2/library/tempfile.html#tempfile.mktemp tempfile.mkstemp() could have also been used, but it also creates a file, and returns an open file descriptor for that. Anyway, since it is in a test case, I don't see that mktemp() would cause a vulnerability issue, so now I use it for brevity and better readability. Done. http://gerrit.cloudera.org:8080/#/c/8368/3/tests/shell/util.py File tests/shell/util.py: http://gerrit.cloudera.org:8080/#/c/8368/3/tests/shell/util.py@121 PS3, Line 121: def restore_shell_history(filepath): > mention that this is a no-op if filepath doesn't exist. Done. -- 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: 4 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 14 Nov 2017 15:34:03 +0000 Gerrit-HasComments: Yes
