Casey Ching has posted comments on this change. Change subject: Source query files from shell. ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/2663/1/shell/impala_shell.py File shell/impala_shell.py: Line 1034: print_to_stderr("Error opening file '%s': %s" % (args, e)) Reducing the scope of this try/except to just the open() call would be nice. As it is now it's hard to tell if some unexpected error could happen and lead to a strange error message thats not really about open() call. Also the file could be explicitly closed. If you want to keep a catch all try/expect around the execution that's fine with me. Line 1110: if not self.imp_client.connected: return False Will the user see a message saying why the file wasn't executed? Line 1111: queries = [ self.sanitise_input(q) for q in self.cmdqueue + queries ] I didn't dig into how cmdqueue works but maybe add a test to check that commands in the queue dont get executed twice if the file being sourced has a source command itself. I'm assuming a sourced file can source another file, is that right? -- To view, visit http://gerrit.cloudera.org:8080/2663 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib05df3e755cd12e9e9562de6b353857940eace03 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson <[email protected]> Gerrit-Reviewer: Casey Ching <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-HasComments: Yes
