Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8549 )
Change subject: IMPALA-1144: Fix exception when CTRL+C on running query in Impala-shell ...................................................................... Patch Set 2: (5 comments) The Python looks fine to me (after David's semicolon comment). I think it'd be good to try to add a regular test for cancellation in the shell. http://gerrit.cloudera.org:8080/#/c/8549/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8549/2//COMMIT_MSG@7 PS2, Line 7: IMPALA-1144: Fix exception when CTRL+C on running query in Impala-shell Perhaps: Fix exception when cancelling query in impala-shell with Ctrl-C "when CTRL+C" reads oddly to me. http://gerrit.cloudera.org:8080/#/c/8549/2//COMMIT_MSG@11 PS2, Line 11: 'Invalid query handle'. nit: reformat this as one paragraph? http://gerrit.cloudera.org:8080/#/c/8549/2//COMMIT_MSG@22 PS2, Line 22: was issued automatically. This happened to historical reasons but not needed nit: "but is not needed" (insert "is") http://gerrit.cloudera.org:8080/#/c/8549/2//COMMIT_MSG@25 PS2, Line 25: Tested manually starting with a select from a random table that has few Can we write a normal test for this? I don't know if it's true, but "select sleep(60000) from functional.whatever" may actually work with cancellation. Or it may not: I haven't tried it! http://gerrit.cloudera.org:8080/#/c/8549/2/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/8549/2/shell/impala_shell.py@485 PS2, Line 485: new_imp_client = self._new_impala_client() does new_imp_client need to be closed? or old_imp_client? -- To view, visit http://gerrit.cloudera.org:8080/8549 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6cefaf1dae78baae238289816a7cb9d210fb38e2 Gerrit-Change-Number: 8549 Gerrit-PatchSet: 2 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Wed, 15 Nov 2017 00:52:20 +0000 Gerrit-HasComments: Yes
