Csaba Ringhofer 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 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/23883/6/shell/impala_shell/impala_shell.py
File shell/impala_shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/23883/6/shell/impala_shell/impala_shell.py@1237
PS6, Line 1237:     if self.show_profiles or status:
              :       try:
              :         out_file = sys.stdout
> Done
IMO it would be nicer to just return early if this is a NOOP and avoid the 
extra indentation.

Actually after looking through the call sites, I think that this function 
should be refactored - the "self.show_profiles or status" check is no longer 
needed, as this function is always called with one of these as true. 'status' 
is not used for anything else and could be removed. This refactor should have 
been done in https://gerrit.cloudera.org/#/c/22085/ - before that 
"self.show_profiles or status" could be actually false.

Btw we could also return early profile is None - I don't know if this is 
actually possible, maybe it would be better if the 2 call sites would ensure 
this too.


http://gerrit.cloudera.org:8080/#/c/23883/7/shell/impala_shell/impala_shell.py
File shell/impala_shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/23883/7/shell/impala_shell/impala_shell.py@1232
PS7, Line 1232: console
This could be updated.



--
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: 7
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-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Fri, 23 Jan 2026 11:25:28 +0000
Gerrit-HasComments: Yes

Reply via email to