Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15345 )
Change subject: IMPALA-9398: Fix shell history duplication when cmdloop breaks ...................................................................... Patch Set 1: (2 comments) Thanks for the fixing this problem! I only have two very minor comments. Thanks! http://gerrit.cloudera.org:8080/#/c/15345/1/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/15345/1/shell/impala_shell.py@1366 PS1, Line 1366: if self.readline.get_current_history_length() == 0: > It's not altogether clear what the significance of current history length b Is it true that when the Impala shell is running and there is an interrupt signal caught, the exception will be caught at https://github.com/apache/impala/blob/master/shell/impala_shell.py#L1859. But since the shell is still alive (https://github.com/apache/impala/blob/master/shell/impala_shell.py#L1855), so shell.cmdloop() will be called again. Is my understanding correct? Thanks! http://gerrit.cloudera.org:8080/#/c/15345/1/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/15345/1/tests/shell/test_shell_interactive.py@473 PS1, Line 473: p = ImpalaShell(vector) Is it true that the reason we use ImpalaShell() here instead of pexpect.spawn() to construct a new instance is because we have splitlines() that helps us splitting the returned lines when ImpalaShell()? Thanks! -- To view, visit http://gerrit.cloudera.org:8080/15345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If4faf46134f44d91e56748642f47d448707db53c Gerrit-Change-Number: 15345 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate <tm...@cloudera.com> Gerrit-Reviewer: David Knupp <dkn...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Tue, 03 Mar 2020 19:25:12 +0000 Gerrit-HasComments: Yes
