You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org> on 2019/12/18 23:10:20 UTC
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14927
Change subject: IMPALA-6393 live_summary and live_progress are not supported in impalarc
......................................................................
IMPALA-6393 live_summary and live_progress are not supported in impalarc
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.
Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86
---
M bin/rat_exclude_files.txt
M shell/option_parser.py
A tests/shell/good_impalarc3
A tests/shell/good_impalarc4
A tests/shell/impalarc_with_error2
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
7 files changed, 96 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/14927/1
--
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: newchange
Gerrit-Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86
Gerrit-Change-Number: 14927
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Bikramjeet Vig, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14927
to look at the new patch set (#2).
Change subject: IMPALA-6393 live_summary and live_progress are not supported in impalarc
......................................................................
IMPALA-6393 live_summary and live_progress are not supported in impalarc
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.
Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86
---
M bin/rat_exclude_files.txt
M shell/option_parser.py
A tests/shell/good_impalarc3
A tests/shell/good_impalarc4
A tests/shell/impalarc_with_error2
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
7 files changed, 95 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/14927/2
--
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: newpatchset
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>
[Impala-ASF-CR] IMPALA-6393: Add support for live summary and live progress in impalarc
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14927 )
Change subject: IMPALA-6393: Add support for live_summary and live_progress in impalarc
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5455/ DRY_RUN=false
--
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: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Jan 2020 21:02:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6393: Add support for live summary and live progress in impalarc
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/14927 )
Change subject: IMPALA-6393: Add support for live_summary and live_progress in impalarc
......................................................................
IMPALA-6393: Add support for live_summary and live_progress in impalarc
This patch adds support for live_summary and live_progress in impalarc.
Testing:
1) Added unit-test cases in test_shell_commandline.py and
test_shell_interactive.py for live_summary and live_progress.
2) Successfully ran all other tests in test_shell_interactive.py and
test_shell_commandline.py
Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86
---
M bin/rat_exclude_files.txt
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
A tests/shell/good_impalarc3
A tests/shell/good_impalarc4
A tests/shell/impalarc_with_error2
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
9 files changed, 93 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/14927/5
--
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: newpatchset
Gerrit-Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86
Gerrit-Change-Number: 14927
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-6393: Add support for live summary and live progress in impalarc
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14927 )
Change subject: IMPALA-6393: Add support for live_summary and live_progress in impalarc
......................................................................
Patch Set 5: Code-Review+2
--
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: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Jan 2020 21:01:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6393: Add support for live summary and live progress in impalarc
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14927 )
Change subject: IMPALA-6393: Add support for live_summary and live_progress in impalarc
......................................................................
Patch Set 5: Verified+1
--
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: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Jan 2020 01:48:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5432/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2020 01:03:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5317/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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, 19 Dec 2019 00:25:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14927/1/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:
http://gerrit.cloudera.org:8080/#/c/14927/1/tests/shell/test_shell_interactive.py@489
PS1, Line 489: #
flake8: E265 block comment should start with '# '
--
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: 1
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: Wed, 18 Dec 2019 23:10:58 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou 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:
(3 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
Will change internal variable names from print_summary and print_progress to live_summary and live_progress respectively.
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
will remove as suggested
http://gerrit.cloudera.org:8080/#/c/14927/1/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:
http://gerrit.cloudera.org:8080/#/c/14927/1/tests/shell/test_shell_interactive.py@489
PS1, Line 489: a
> flake8: E265 block comment should start with '# '
remove this line
--
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-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 Jan 2020 01:22:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6393: Add support for live summary and live progress in impalarc
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14927 )
Change subject: IMPALA-6393: Add support for live_summary and live_progress in impalarc
......................................................................
IMPALA-6393: Add support for live_summary and live_progress in impalarc
This patch adds support for live_summary and live_progress in impalarc.
Testing:
1) Added unit-test cases in test_shell_commandline.py and
test_shell_interactive.py for live_summary and live_progress.
2) Successfully ran all other tests in test_shell_interactive.py and
test_shell_commandline.py
Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86
Reviewed-on: http://gerrit.cloudera.org:8080/14927
Reviewed-by: Bikramjeet Vig <bi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M bin/rat_exclude_files.txt
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
A tests/shell/good_impalarc3
A tests/shell/good_impalarc4
A tests/shell/impalarc_with_error2
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
9 files changed, 93 insertions(+), 15 deletions(-)
Approvals:
Bikramjeet Vig: Looks good to me, approved
Impala Public Jenkins: Verified
--
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: merged
Gerrit-Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86
Gerrit-Change-Number: 14927
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5316/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 1
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: Wed, 18 Dec 2019 23:41:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou 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 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14927/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14927/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-6393 live_summary and live_progress are not supported in impalarc
> IMPALA-6393: Add support for live_summary and live_progress in impalarc
will fix the commit message as suggested
http://gerrit.cloudera.org:8080/#/c/14927/3//COMMIT_MSG@9
PS3, Line 9: The internal variables for live_summary and live_progress are defined as print_summary
: and print_progress in impala_shell. When reading configuration parameters from the
: config file, it save configurations as dictionary with external name as key.
: This cause live_summary and live_progress in impalarc do not take effect in impala_shell.
:
: To fix the issue, change internal variable names same as external names for option
: live_summary and live_progress.
:
: Unit test:
: 1) manually verified live_summary and live_progress are supported in impalarc
: 2) add some unit-test cases in test_shell_commandline.py and test_shell_interactive.py
: for option live_summary and live_progress read from impalarc.
: 3) passed all test cases for tests/shell/test_shell_interactive.py and
: tests/shell/test_shell_commandline.py
> we dont really need implementation details in the commit message unless som
will fix the commit message as suggested
http://gerrit.cloudera.org:8080/#/c/14927/3/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:
http://gerrit.cloudera.org:8080/#/c/14927/3/tests/shell/test_shell_interactive.py@500
PS3, Line 500: def test_live_option_configuration(self, vector):
: """Test the optional configuration file with live_progress and live_summary."""
: # Positive tests
: # set live_summary and live_progress as True with config file
: rcfile_path = os.path.join(QUERY_FILE_PATH, 'good_impalarc3')
: args = ['--config_file=%s' % rcfile_path]
: cmds = "set all;"
: result = run_impala_shell_interactive(vector, cmds, shell_args=args)
: assert 'WARNING:' not in result.stderr, \
: "A valid config file should not trigger any warning: {0}".format(result.stderr)
: assert "\tLIVE_SUMMARY: True" in result.stdout
: assert "\tLIVE_PROGRESS: True" in result.stdout
:
: # set live_summary and live_progress as False with config file
: rcfile_path = os.path.join(QUERY_FILE_PATH, 'good_impalarc4')
: args = ['--config_file=%s' % rcfile_path]
: result = run_impala_shell_interactive(vector, cmds, shell_args=args)
: assert 'WARNING:' not in result.stderr, \
: "A valid config file should not trigger any warning: {0}".format(result.stderr)
: assert "\tLIVE_SUMMARY: False" in result.stdout
: assert "\tLIVE_PROGRESS: False" in result.stdout
: # override options in config file through command line arguments
: args = ['--live_progress', '--live_summary', '--config_file=%s' % rcfile_path]
: result = run_impala_shell_interactive(vector, cmds, shell_args=args)
: assert "\tLIVE_SUMMARY: True" in result.stdout
: assert "\tLIVE_PROGRESS: True" in result.stdout
> I think we can skip the interactive tests and confirm that the values are b
Live Summary and Live Progress are available for interactive mode only so we have to run test for "Live Summary" and "Live Progress" setting in interactive tests.
--
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: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2020 01:20:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6393: Add support for live summary and live progress in impalarc
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14927 )
Change subject: IMPALA-6393: Add support for live_summary and live_progress in impalarc
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5497/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Jan 2020 20:37:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6393 Add support for live summary and live progress in impalarc
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/14927 )
Change subject: IMPALA-6393 Add support for live_summary and live_progress in impalarc
......................................................................
IMPALA-6393 Add support for live_summary and live_progress in impalarc
This patch adds support for live_summary and live_progress in impalarc.
Testing:
1) Added unit-test cases in test_shell_commandline.py and test_shell_interactive.py
for live_summary and live_progress.
2) Successfully ran all other tests in test_shell_interactive.py and test_shell_commandline.py
Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86
---
M bin/rat_exclude_files.txt
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
A tests/shell/good_impalarc3
A tests/shell/good_impalarc4
A tests/shell/impalarc_with_error2
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
9 files changed, 93 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/14927/4
--
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: newpatchset
Gerrit-Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86
Gerrit-Change-Number: 14927
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
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 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14927/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14927/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-6393 live_summary and live_progress are not supported in impalarc
IMPALA-6393: Add support for live_summary and live_progress in impalarc
http://gerrit.cloudera.org:8080/#/c/14927/3//COMMIT_MSG@9
PS3, Line 9: The internal variables for live_summary and live_progress are defined as print_summary
: and print_progress in impala_shell. When reading configuration parameters from the
: config file, it save configurations as dictionary with external name as key.
: This cause live_summary and live_progress in impalarc do not take effect in impala_shell.
:
: To fix the issue, change internal variable names same as external names for option
: live_summary and live_progress.
:
: Unit test:
: 1) manually verified live_summary and live_progress are supported in impalarc
: 2) add some unit-test cases in test_shell_commandline.py and test_shell_interactive.py
: for option live_summary and live_progress read from impalarc.
: 3) passed all test cases for tests/shell/test_shell_interactive.py and
: tests/shell/test_shell_commandline.py
we dont really need implementation details in the commit message unless something really important needs to be pointed out. You can just write:
This patch adds support for live_summary and live_progress in impalarc.
Testing:
Added tests to test_shell_commandline.py
Successfully ran all other tests in test_shell_interactive.py and test_shell_commandline.py
http://gerrit.cloudera.org:8080/#/c/14927/3/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:
http://gerrit.cloudera.org:8080/#/c/14927/3/tests/shell/test_shell_interactive.py@500
PS3, Line 500: def test_live_option_configuration(self, vector):
: """Test the optional configuration file with live_progress and live_summary."""
: # Positive tests
: # set live_summary and live_progress as True with config file
: rcfile_path = os.path.join(QUERY_FILE_PATH, 'good_impalarc3')
: args = ['--config_file=%s' % rcfile_path]
: cmds = "set all;"
: result = run_impala_shell_interactive(vector, cmds, shell_args=args)
: assert 'WARNING:' not in result.stderr, \
: "A valid config file should not trigger any warning: {0}".format(result.stderr)
: assert "\tLIVE_SUMMARY: True" in result.stdout
: assert "\tLIVE_PROGRESS: True" in result.stdout
:
: # set live_summary and live_progress as False with config file
: rcfile_path = os.path.join(QUERY_FILE_PATH, 'good_impalarc4')
: args = ['--config_file=%s' % rcfile_path]
: result = run_impala_shell_interactive(vector, cmds, shell_args=args)
: assert 'WARNING:' not in result.stderr, \
: "A valid config file should not trigger any warning: {0}".format(result.stderr)
: assert "\tLIVE_SUMMARY: False" in result.stdout
: assert "\tLIVE_PROGRESS: False" in result.stdout
: # override options in config file through command line arguments
: args = ['--live_progress', '--live_summary', '--config_file=%s' % rcfile_path]
: result = run_impala_shell_interactive(vector, cmds, shell_args=args)
: assert "\tLIVE_SUMMARY: True" in result.stdout
: assert "\tLIVE_PROGRESS: True" in result.stdout
I think we can skip the interactive tests and confirm that the values are being set properly in the test_shell_commandline.
Also include this case there "override options in config file through command line arguments"
--
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: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jan 2020 23:00:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/14927 )
Change subject: IMPALA-6393 live_summary and live_progress are not supported in impalarc
......................................................................
IMPALA-6393 live_summary and live_progress are not supported in impalarc
The internal variables for live_summary and live_progress are defined as print_summary
and print_progress in impala_shell. When reading configuration parameters from the
config file, it save configurations as dictionary with external name as key.
This cause live_summary and live_progress in impalarc do not take effect in impala_shell.
To fix the issue, change internal variable names same as external names for option
live_summary and live_progress.
Unit test:
1) manually verified live_summary and live_progress are supported in impalarc
2) add some unit-test cases in test_shell_commandline.py and test_shell_interactive.py
for option live_summary and live_progress read from impalarc.
3) passed all test cases for tests/shell/test_shell_interactive.py and
tests/shell/test_shell_commandline.py
Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86
---
M bin/rat_exclude_files.txt
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
A tests/shell/good_impalarc3
A tests/shell/good_impalarc4
A tests/shell/impalarc_with_error2
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
9 files changed, 93 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/14927/3
--
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: newpatchset
Gerrit-Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86
Gerrit-Change-Number: 14927
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
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
[Impala-ASF-CR] IMPALA-6393 Add support for live summary and live progress in impalarc
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14927 )
Change subject: IMPALA-6393 Add support for live_summary and live_progress in impalarc
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14927/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14927/4//COMMIT_MSG@7
PS4, Line 7: IMPALA-6393 Add support for live_summary and live_progress in impalarc
nit: add a colon here like
IMPALA-6393: Add support for live_summary and live_progress in impalarc
http://gerrit.cloudera.org:8080/#/c/14927/4//COMMIT_MSG@12
PS4, Line 12: 1) Added unit-test cases in test_shell_commandline.py and test_shell_interactive.py
: for live_summary and live_progress.
: 2) Successfully ran all other tests in test_shell_interactive.py and test_shell_commandline.py
nit: can you please wrap text for commit messages at 73 characters. (notice the vertical yellow line)
--
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: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Jan 2020 19:32:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6393 Add support for live summary and live progress in impalarc
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14927 )
Change subject: IMPALA-6393 Add support for live_summary and live_progress in impalarc
......................................................................
Patch Set 4:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/5441/ : Initial code review checks failed. See linked job for details on the failure.
--
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: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Jan 2020 02:33:51 +0000
Gerrit-HasComments: No