Tianyi Wang has posted comments on this change. Change subject: IMPALA-992: Rerun past queries from history in shell ......................................................................
Patch Set 4: (12 comments) http://gerrit.cloudera.org:8080/#/c/7674/2//COMMIT_MSG Commit Message: PS2, Line 7: IMPALA > nit: capitalize Done PS2, Line 9: @ > What's the reason for picking "@" over "!"? The latter is used in shells an Currently "!" is used to run shell commands in impala-shell. PS2, Line 12: _length, -1]. Negavie val > Can you give a usage example? Does the history count backwards or forwards More example and explanation is given. http://gerrit.cloudera.org:8080/#/c/7674/2/shell/impala_shell.py File shell/impala_shell.py: PS2, Line 79: object > Why is this needed? The direct reason is that "super" in line 1164 cannot be used with an old style class. And old style class is not recommended after python 2.1. https://stackoverflow.com/questions/13816849/old-style-and-new-style-classes-in-python-2-7 Line 1073: print_to_stderr("The readline module was either not found or disabled. Command " > nit: You can remove the else and unindent the block, just like you do in th Done PS2, Line 1080: one' a > Not your change, but: Done PS2, Line 1090: > Format on two lines to make it easier to read. Done PS2, Line 1102: if not (0 < cmd_idx <= history_length or -history_length <= cmd_idx < 0): > Please use explicit indexing or kwarg-style format: "{}".format(str) is not Done PS2, Line 1102: if not (0 < cmd_idx <= history_length or -history_length <= cmd_idx < 0): > long line Done Line 1107: cmd_idx += history_length + 1 > Does it always have a semicolon? .rstrip(";") might be more robust and clea Done Line 1156: # The history file is not writable, disable readline. > Can you mention that this method will perform transformations? Done PS2, Line 1157: > what will 'line' contain? One more line of comment is added. -- To view, visit http://gerrit.cloudera.org:8080/7674 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifc28e8ce07845343267224c3b9ccb71b29a524d2 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi Wang <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-HasComments: Yes
