Balazs Hevele 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 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/23883/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23883/3//COMMIT_MSG@13 PS3, Line 13: > nit: use spaces instead of tabs Done http://gerrit.cloudera.org:8080/#/c/23883/3/shell/impala_shell/impala_shell.py File shell/impala_shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/23883/3/shell/impala_shell/impala_shell.py@1242 PS3, Line 1242: if out_file: : print(data, file=out_file) : else: : print(data) : > It seems simpler to always use write or print and use sys.stdout in case th Done http://gerrit.cloudera.org:8080/#/c/23883/3/shell/impala_shell/option_parser.py File shell/impala_shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/23883/3/shell/impala_shell/option_parser.py@211 PS3, Line 211: "queries will be appended to the same file") > nit: +1 space Done http://gerrit.cloudera.org:8080/#/c/23883/3/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/23883/3/tests/shell/test_shell_commandline.py@427 PS3, Line 427: # This regex helps us uniquely identify a profile. > Can you also test the case when -p/show_profiles option is used? Done http://gerrit.cloudera.org:8080/#/c/23883/3/tests/shell/test_shell_commandline.py@434 PS3, Line 434: # We expect no results in stdout > nit: when breaking lines, we usually use 4 spaces indentation. e.g see line Done http://gerrit.cloudera.org:8080/#/c/23883/3/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/23883/3/tests/shell/test_shell_interactive.py@247 PS3, Line 247: self._expect_with_cmd(proc, "set profile_output=/tmp/profile.txt", vector) > It would be nice to test that the config is actually used. I expect somethi Done -- 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: 4 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-Comment-Date: Thu, 22 Jan 2026 10:48:13 +0000 Gerrit-HasComments: Yes
