Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/23883 )
Change subject: IMPALA-572 impala-shell: add option to write profiles to a file ...................................................................... Patch Set 5: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/23883/5/shell/impala_shell/impala_shell.py File shell/impala_shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/23883/5/shell/impala_shell/impala_shell.py@1241 PS5, Line 1241: def profile_print(data): : if out_file: : print(data, file=out_file) : else: : print(data) You could use a variable to store the target for printing with a default value to sys.stdout, this way there's no need for profile_print, the target could be changed in L1239 http://gerrit.cloudera.org:8080/#/c/23883/5/shell/impala_shell/impala_shell.py@2472 PS5, Line 2472: # Make sure the given file can be opened for writing. This will also clear nit: please move the comment and the previous block's comment above, and give a common description about these opens http://gerrit.cloudera.org:8080/#/c/23883/5/shell/impala_shell/impala_shell.py@2474 PS5, Line 2474: open(options.profile_output, 'w') I know it's mainly from the previous code part, but it seems a good time to move them to a "with open" statement. -- To view, visit http://gerrit.cloudera.org:8080/23883 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8ce4ddcf013392b3c4d66941f07fb90f9c90c3c Gerrit-Change-Number: 23883 Gerrit-PatchSet: 5 Gerrit-Owner: Balazs Hevele <[email protected]> Gerrit-Reviewer: Balazs Hevele <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Comment-Date: Thu, 22 Jan 2026 14:18:53 +0000 Gerrit-HasComments: Yes
