You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org> on 2020/01/09 19:08:23 UTC

[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc

Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14927 )

Change subject: IMPALA-6393 live_summary and live_progress are not supported in impalarc
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14927/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14927/2//COMMIT_MSG@9
PS2, Line 9: impala_shell main function call function get_config_from_file() to read impalarc file. The
           : function get_config_from_file() save and return the shell options in one dictionary
           : 'shell_options' with the option name as key. impala_shell then update the default option
           : values in 'impala_shell_defaults' with the returned shell options dictionary. But the
           : 'impala_shell_defaults' is defined with option 'dest' as key, instead of option name itself.
           : There are some options for which option name and option dest are different. For example,
           : dest of 'live_summary' is defined as 'print_summary', dest of 'live_progress' is defined as
           : 'print_progress'. This cause option values in impala_shell_defaults are not updated with
           : the values specified in config file if option name and option dest are different. This is
           : the root cause why live_summary and live_progress in impalarc do not take effect in impala_shell.
           : 
           : To fix the issue, modify function get_config_from_file() to save the options with 'dest' as key.
           : Add some unit-test cases in test_shell_commandline.py and test_shell_interactive.py.
seems like print_summary is just an internal variable name. I think we can just switch that to live_summary instead and avoid changes that map print_summary to live_summary


http://gerrit.cloudera.org:8080/#/c/14927/2/tests/shell/impalarc_with_error2
File tests/shell/impalarc_with_error2:

http://gerrit.cloudera.org:8080/#/c/14927/2/tests/shell/impalarc_with_error2@7
PS2, Line 7: #quiet=false
           : #ldap=true
           : #delimited=true
nit: remove here and in other impalarc files too



-- 
To view, visit http://gerrit.cloudera.org:8080/14927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86
Gerrit-Change-Number: 14927
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Jan 2020 19:08:23 +0000
Gerrit-HasComments: Yes