[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. The default path of the global config file can be changed by setting the $IMPALA_SHELL_GLOBAL_CONFIG_FILE environment variable. Note that the options set in the user config file take precedence over those in the global config file. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Reviewed-on: http://gerrit.cloudera.org:8080/13313 Reviewed-by: Bikramjeet Vig Tested-by: Impala Public Jenkins --- 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_impalarc2 M tests/shell/impalarc_with_query_options M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 9 files changed, 112 insertions(+), 36 deletions(-) Approvals: Bikramjeet Vig: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 15 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 14: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 14 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 30 May 2019 03:59:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 14: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3429/ : 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/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 14 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 May 2019 22:09:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 13: (4 comments) http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py@479 PS12, Line 479: def test_global_config_file(s > we dont need this test to execute serially so you can remove it Removed it. Just curious why we don't need it though as the test below also tests config files and it runs serially. http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py@483 PS12, Line 483: > this is not being verified Thanks, I added a verification. http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py@487 PS12, Line 487: assert 'Query: select 2' in result.std > what is this verifying? This is verifying that a valid global impalarc file should not trigger any warnings in the shell. A similar verification is done for the user config. http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_interactive.py@434 PS12, Line 434: # verify that query options under [impala] override those under [impala.query_options] > nit: add a comment above this mentioning that you are verifying id the quer Done -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 13 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 May 2019 17:04:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Hello Fredy Wijaya, Todd Lipcon, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13313 to look at the new patch set (#13). Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. The default path of the global config file can be changed by setting the $IMPALA_SHELL_GLOBAL_CONFIG_FILE environment variable. Note that the options set in the user config file take precedence over those in the global config file. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- 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_impalarc2 M tests/shell/impalarc_with_query_options M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 9 files changed, 109 insertions(+), 34 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/13 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 13 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 14: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4345/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 14 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 May 2019 21:58:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 14: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 14 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 May 2019 21:57:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 13: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3423/ : 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/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 13 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 May 2019 17:44:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Hello Fredy Wijaya, Todd Lipcon, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13313 to look at the new patch set (#14). Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. The default path of the global config file can be changed by setting the $IMPALA_SHELL_GLOBAL_CONFIG_FILE environment variable. Note that the options set in the user config file take precedence over those in the global config file. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- 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_impalarc2 M tests/shell/impalarc_with_query_options M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 9 files changed, 112 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/14 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 14 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 13: (3 comments) http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py@479 PS12, Line 479: def test_global_config_file(s > Removed it. Just curious why we don't need it though as the test below also Thanks for pointing that out, I looked into it and it seems like that was added because the previous version of that test invalidated metadata which would have affected other tests running in parallel. Since that is no longer true, can you please remove it from there as well? http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py@487 PS12, Line 487: assert 'Query: select 2' in result.std > This is verifying that a valid global impalarc file should not trigger any What are the cases where warnings would be triggered? If there is a case specific to the global impalarc, then can you add the full warning text so that we have more context as to what we are asserting. If there is no specific warning message and you are sure nothing else will trigger a warning then in that case can you add an error text to it which is printed if this assert fails. For eg. assert 'WARNING:' not in result.stderr, "Error text that refers to what was expected but not encountered giving more context as to why this assert was added. also print the full result.stdout here so that it is easy to debug in case it fails" http://gerrit.cloudera.org:8080/#/c/13313/13/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/13313/13/tests/shell/test_shell_interactive.py@434 PS13, Line 434: v nit: upper case -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 13 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 May 2019 20:57:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 14: (3 comments) http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py@479 PS12, Line 479: def test_global_config_file(s > Thanks for pointing that out, I looked into it and it seems like that was a Done http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py@487 PS12, Line 487: "A valid config file should not trig > What are the cases where warnings would be triggered? If there is a case sp Done http://gerrit.cloudera.org:8080/#/c/13313/13/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/13313/13/tests/shell/test_shell_interactive.py@434 PS13, Line 434: V > nit: upper case Done -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 14 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 May 2019 21:27:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 12: (4 comments) http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py@479 PS12, Line 479: @pytest.mark.execute_serially we dont need this test to execute serially so you can remove it http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py@483 PS12, Line 483: query options this is not being verified http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_commandline.py@487 PS12, Line 487: assert 'WARNING:' not in result.stderr what is this verifying? http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/13313/12/tests/shell/test_shell_interactive.py@434 PS12, Line 434: assert "\tDEFAULT_FILE_FORMAT: avro" in result.stdout nit: add a comment above this mentioning that you are verifying id the query option under [impala] overrides the one under [impala.query_options] -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 12 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 May 2019 00:34:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3411/ : 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/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 11 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 28 May 2019 19:09:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3412/ : 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/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 12 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 28 May 2019 19:08:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Hello Fredy Wijaya, Todd Lipcon, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13313 to look at the new patch set (#11). Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. The default path of the global config file can be changed by setting the $IMPALA_SHELL_GLOBAL_CONFIG_FILE environment variable. Note that the options set in the user config file take precedence over those in the global config file. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- 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_impalarc2 M tests/shell/impalarc_with_query_options M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 9 files changed, 106 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/11 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 11 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Hello Fredy Wijaya, Todd Lipcon, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13313 to look at the new patch set (#12). Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. The default path of the global config file can be changed by setting the $IMPALA_SHELL_GLOBAL_CONFIG_FILE environment variable. Note that the options set in the user config file take precedence over those in the global config file. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- 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_impalarc2 M tests/shell/impalarc_with_query_options M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 9 files changed, 104 insertions(+), 34 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/12 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 12 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 10: (3 comments) http://gerrit.cloudera.org:8080/#/c/13313/10/shell/impala_shell_config_defaults.py File shell/impala_shell_config_defaults.py: http://gerrit.cloudera.org:8080/#/c/13313/10/shell/impala_shell_config_defaults.py@20 PS10, Line 20: # defaults for OptionParser options stored in dict > nit: we'll need to update this since we are now storing a default not used Done http://gerrit.cloudera.org:8080/#/c/13313/10/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/13313/10/tests/shell/test_shell_commandline.py@479 PS10, Line 479: # @pytest.mark.execute_serially > nit: remove comment Done http://gerrit.cloudera.org:8080/#/c/13313/10/tests/shell/test_shell_commandline.py@480 PS10, Line 480: def test_global_config_file(self, vector): > I think we only need the following tests: Done -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 10 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 28 May 2019 18:23:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 10: (3 comments) http://gerrit.cloudera.org:8080/#/c/13313/10/shell/impala_shell_config_defaults.py File shell/impala_shell_config_defaults.py: http://gerrit.cloudera.org:8080/#/c/13313/10/shell/impala_shell_config_defaults.py@20 PS10, Line 20: # defaults for OptionParser options stored in dict nit: we'll need to update this since we are now storing a default not used by OptionParser http://gerrit.cloudera.org:8080/#/c/13313/10/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/13313/10/tests/shell/test_shell_commandline.py@479 PS10, Line 479: # @pytest.mark.execute_serially nit: remove comment http://gerrit.cloudera.org:8080/#/c/13313/10/tests/shell/test_shell_commandline.py@480 PS10, Line 480: def test_global_config_file(self, vector): I think we only need the following tests: - shell uses options and query options in global config - options and query options in global config get overridden by those in user config - options in global config get overridden by command line options - specified global config file does not exist Also add a test in test_query_option_configuration which verifies that query_options under [impala] override the query_options defined under [impala.query_options] -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 10 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 25 May 2019 00:38:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3372/ : 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/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 10 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 May 2019 00:42:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Hello Fredy Wijaya, Todd Lipcon, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13313 to look at the new patch set (#10). Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. The default path of the global config file can be changed by setting the $IMPALA_SHELL_GLOBAL_CONFIG_FILE environment variable. Note that the options set in the user config file take precedence over those in the global config file. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- 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_impalarc2 M tests/shell/test_shell_commandline.py M tests/shell/util.py 7 files changed, 111 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/10 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 10 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/13313/9/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/13313/9/tests/shell/test_shell_commandline.py@480 PS9, Line 480: test_impalarc > nit: rename to test_global_config_file to be consistent with the next test Done http://gerrit.cloudera.org:8080/#/c/13313/9/tests/shell/test_shell_commandline.py@511 PS9, Line 511: > It seems like we can set query options through either [impala] or [impala.q It turns out the order is: query_options from command line override query_options from impalarc shell options which override options from impalarc query options -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 9 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 23 May 2019 20:38:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/13313/9/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/13313/9/tests/shell/test_shell_commandline.py@480 PS9, Line 480: test_impalarc nit: rename to test_global_config_file to be consistent with the next test http://gerrit.cloudera.org:8080/#/c/13313/9/tests/shell/test_shell_commandline.py@511 PS9, Line 511: It seems like we can set query options through either [impala] or [impala.query_options] notations. Can you take a look at what the precedence order is if both are defined. It would be good to also add tests that make sure that the order is honored and also file a jira to update the docs accordingly -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 9 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 23 May 2019 00:53:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3300/ : 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/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 9 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 20 May 2019 23:10:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/13313/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13313/8//COMMIT_MSG@9 PS8, Line 9: Currently, impalarc files can be specified on a per-user basis : (stored in ~/.impalarc), and they aren't created by default. The : Impala shell should pick up /etc/impalarc as well, in addition : to the user-specific configurations. : : The intent here is to allow a "global" configuration of the shell : by a system administrator. The default path of the global config : file can be changed by setting the $IMPALA_SHELL_GLOBAL_CONFIG_FILE : environment variable. > can you also add that the configs set in the user specified config file tak Done -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 9 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 20 May 2019 22:47:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Hello Fredy Wijaya, Todd Lipcon, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13313 to look at the new patch set (#9). Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. The default path of the global config file can be changed by setting the $IMPALA_SHELL_GLOBAL_CONFIG_FILE environment variable. Note that the options set in the user config file take precedence over those in the global config file. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- 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_impalarc2 M tests/shell/test_shell_commandline.py M tests/shell/util.py 7 files changed, 100 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/9 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 9 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/13313/8/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/13313/8/shell/impala_shell.py@1613 PS8, Line 1613: There are two types of options: shell options and query_options. Both can be set in the : command line, which override the options set in the user config file (~/.impalarc) and : the global config file (/etc/impalarc) > can you rephrase this and mention the order of precedence explicitly Done http://gerrit.cloudera.org:8080/#/c/13313/8/shell/impala_shell.py@1653 PS8, Line 1653: options defaults overwritten by those in config file > nit: update comment Done http://gerrit.cloudera.org:8080/#/c/13313/8/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/13313/8/tests/shell/test_shell_commandline.py@527 PS8, Line 527: assert 'DEFAULT_FILE_FORMAT: parquet' in result.stdout > can you also incorporate adding a change to the query options like this. So Done -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 8 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 20 May 2019 22:43:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/13313/8/shell/option_parser.py File shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/13313/8/shell/option_parser.py@288 PS8, Line 288: and option.help is not SUPPRESS_HELP: : # don't want to print default value for help or options : # without help text > remove the changes to this file, they are not relevant to the patch I agree that it's probably not relevant. However, it does fix a bug though. If an option is added to the parser with SUPPRESS_HELP, it would still show up in the help text, unless you add this additional check. Should I separate out this change to a separate patch? -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 8 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 20 May 2019 20:05:07 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 8: (5 comments) Looks good, just a few nits http://gerrit.cloudera.org:8080/#/c/13313/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13313/8//COMMIT_MSG@9 PS8, Line 9: Currently, impalarc files can be specified on a per-user basis : (stored in ~/.impalarc), and they aren't created by default. The : Impala shell should pick up /etc/impalarc as well, in addition : to the user-specific configurations. : : The intent here is to allow a "global" configuration of the shell : by a system administrator. The default path of the global config : file can be changed by setting the $IMPALA_SHELL_GLOBAL_CONFIG_FILE : environment variable. can you also add that the configs set in the user specified config file take precedence over the global config http://gerrit.cloudera.org:8080/#/c/13313/8/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/13313/8/shell/impala_shell.py@1613 PS8, Line 1613: There are two types of options: shell options and query_options. Both can be set in the : command line, which override the options set in the user config file (~/.impalarc) and : the global config file (/etc/impalarc) can you rephrase this and mention the order of precedence explicitly http://gerrit.cloudera.org:8080/#/c/13313/8/shell/impala_shell.py@1653 PS8, Line 1653: options defaults overwritten by those in config file nit: update comment http://gerrit.cloudera.org:8080/#/c/13313/8/shell/option_parser.py File shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/13313/8/shell/option_parser.py@288 PS8, Line 288: and option.help is not SUPPRESS_HELP: : # don't want to print default value for help or options : # without help text remove the changes to this file, they are not relevant to the patch http://gerrit.cloudera.org:8080/#/c/13313/8/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/13313/8/tests/shell/test_shell_commandline.py@527 PS8, Line 527: assert 'DEFAULT_FILE_FORMAT: parquet' in result.stdout can you also incorporate adding a change to the query options like this. So that we know both shell and query options are being overloaded -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 8 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 20 May 2019 20:00:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 8: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 8 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 20 May 2019 15:50:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3284/ : 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/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 8 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 18 May 2019 01:16:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 7: > Patch Set 7: Code-Review+1 > > There's a rat check failure, you need to exclude good_impalarc2 in > https://github.com/apache/impala/blob/master/bin/rat_exclude_files.txt > > I'll let Bikramjeet to take another look at it for the +2. Done. Added good_impalarc2 to rat_exclude_files.txt -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 7 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 18 May 2019 00:07:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Hello Fredy Wijaya, Todd Lipcon, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13313 to look at the new patch set (#8). Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. The default path of the global config file can be changed by setting the $IMPALA_SHELL_GLOBAL_CONFIG_FILE environment variable. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- 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_impalarc2 M tests/shell/test_shell_commandline.py M tests/shell/util.py 7 files changed, 84 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/8 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 8 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 7: Code-Review+1 There's a rat check failure, you need to exclude good_impalarc2 in https://github.com/apache/impala/blob/master/bin/rat_exclude_files.txt I'll let Bikramjeet to take another look at it for the +2. -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 7 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 17 May 2019 23:41:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 7: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/3275/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 7 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 17 May 2019 22:44:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Hello Fredy Wijaya, Todd Lipcon, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13313 to look at the new patch set (#7). Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. The default path of the global config file can be changed by setting the $IMPALA_SHELL_GLOBAL_CONFIG_FILE environment variable. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- M shell/impala_shell.py M shell/impala_shell_config_defaults.py M shell/option_parser.py A tests/shell/good_impalarc2 M tests/shell/test_shell_commandline.py M tests/shell/util.py 6 files changed, 83 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/7 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 7 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/13313/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13313/6//COMMIT_MSG@9 PS6, Line 9: Currently, impalarc files can be specified on a per-user basis : (stored in ~/.impalarc), and they aren't created by default. The : Impala shell should pick up /etc/impalarc as well, in addition : to the user-specific configurations. mention the IMPALA_SHELL_GLOBAL_CONFIG_FILE here. http://gerrit.cloudera.org:8080/#/c/13313/6/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/13313/6/shell/impala_shell.py@159 PS6, Line 159: GLOBAL_CONFIG_DEFAULT_PATH = "/etc/impalarc" i believe we still can define the default here: https://github.com/apache/impala/blob/master/shell/impala_shell_config_defaults.py My understanding is impala_shell_config_defaults contains all the default values (not just default argument values) -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 6 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 17 May 2019 21:53:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 5: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/3271/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 5 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 17 May 2019 21:35:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/13313/6/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/13313/6/shell/impala_shell.py@1629 PS6, Line 1629: o flake8: E126 continuation line over-indented for hanging indent -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 6 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 17 May 2019 21:23:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Hello Fredy Wijaya, Todd Lipcon, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13313 to look at the new patch set (#6). Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- M shell/impala_shell.py M shell/option_parser.py A tests/shell/good_impalarc2 M tests/shell/test_shell_commandline.py M tests/shell/util.py 5 files changed, 83 insertions(+), 27 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/6 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 6 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/13313/5/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/13313/5/shell/impala_shell.py@1629 PS5, Line 1629: o flake8: E126 continuation line over-indented for hanging indent -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 5 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 17 May 2019 21:16:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 5: (1 comment) > Patch Set 4: > > (1 comment) http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py File shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py@260 PS1, Line 260: help=SUPPRESS_HELP > +1 on IMPALA_SHELL_GLOBAL_CONFIG_FILE since this is an Impala shell specifi Done -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 5 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 17 May 2019 21:16:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Hello Fredy Wijaya, Todd Lipcon, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13313 to look at the new patch set (#5). Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- M shell/impala_shell.py M shell/option_parser.py A tests/shell/good_impalarc2 M tests/shell/test_shell_commandline.py M tests/shell/util.py 5 files changed, 84 insertions(+), 27 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/5 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 5 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py File shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py@260 PS1, Line 260: help=SUPPRESS_HELP > One potential use case that maybe useful is is to specify a different locat This is one of the ways we thought of while coming up with this solution, so I am completely on board if you think this use case will be common. The only reason I am usually skeptical about adding env variables is that, if you are sourcing a bunch of them in and the naming is ambiguous then there can be some conflicting ones and it can get difficult to manage. We will have to be super careful about naming this so its super obvious what it is used for, how about IMPALA_SHELL_GLOBAL_CONFIG_FILE. Also if we want users to set this then we might want to expose this somewhere in the help text and make sure we update the impala docs. -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 4 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 16 May 2019 19:56:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py File shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py@260 PS1, Line 260: help=SUPPRESS_HELP > This is one of the ways we thought of while coming up with this solution, s +1 on IMPALA_SHELL_GLOBAL_CONFIG_FILE since this is an Impala shell specific config. -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 4 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 16 May 2019 19:58:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 4: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/3250/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 4 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 16 May 2019 18:52:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. (
http://gerrit.cloudera.org:8080/13313 )
Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config
..
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py
File shell/impala_shell.py:
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py@1628
PS1, Line 1628: test_global_config = os.path.expanduser(options.global_config)
: if os.path.isfile(test_global_config):
: # Always output the source of the global config if verbose
: if options.verbose:
: print_to_stderr(
: "Loading in options from global config file: %s \n" %
test_global_config)
: if test_global_config != default_global_config:
: default_global_config = test_global_config
: elif test_global_config != default_global_config:
: print_to_stderr('%s not found.\n' % test_global_config)
: sys.exit(1)
:
: # use path to custom file specified by user in config_file
option
: input_config = os.path.expanduser(options.config_file)
: # verify input_config, if found
: if input_config != default_user_config:
: if os.path.isfile(input_config):
: if options.verbose:
: print_to_stderr("Loading in options from config file:
%s \n" % input_config)
: # command
> One confusion I had earlier was particularly this line: input_config != use
Ah I see what you're getting at, and I agree. Thanks!
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py@1656
PS1, Line 1656:
> nit: formatting is a bit odd. In Python, we usually indent parser.option_li
Done
--
To view, visit http://gerrit.cloudera.org:8080/13313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816
Gerrit-Change-Number: 13313
Gerrit-PatchSet: 4
Gerrit-Owner: Ethan Xue
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Ethan Xue
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Thu, 16 May 2019 17:56:01 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Hello Fredy Wijaya, Todd Lipcon, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13313 to look at the new patch set (#4). Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- M shell/impala_shell.py M shell/impala_shell_config_defaults.py M shell/option_parser.py A tests/shell/good_impalarc2 M tests/shell/test_shell_commandline.py 5 files changed, 78 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/4 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 4 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/13313 )
Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config
..
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13313/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13313/3//COMMIT_MSG@9
PS3, Line 9: Currently, impalarc files can be specified on a per-user basis
(stored in ~/.impalarc), and they aren't created by default. The Impala shell
should pick up /etc/impalarc as well, in addition to the user-specific
configurations.
:
: The intent here is to allow a "global" configuration of the shell
by a system administrator.
nit: we should try to keep it within 72-char width
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py
File shell/impala_shell.py:
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py@1628
PS1, Line 1628: test_global_config = os.path.expanduser(options.global_config)
: if os.path.isfile(test_global_config):
: # Always output the source of the global config if verbose
: if options.verbose:
: print_to_stderr(
: "Loading in options from global config file: %s \n" %
test_global_config)
: if test_global_config != default_global_config:
: default_global_config = test_global_config
: elif test_global_config != default_global_config:
: print_to_stderr('%s not found.\n' % test_global_config)
: sys.exit(1)
:
: # use path to custom file specified by user in config_file
option
: input_config = os.path.expanduser(options.config_file)
: # verify input_config, if found
: if os.path.isfile(input_config) and input_config !=
default_user_config:
: if options.verbose:
: print_to_stderr("Loading in options from config file: %s
\n" % input_config)
: # command line overrides loading ~/.impalarc
: default_use
> Thanks for the heads up. I tried my best to make it more readable. It does
One confusion I had earlier was particularly this line: input_config !=
user_config: to mean file not found. I think this can be made more readable
like below:
if input_config != user_config:
if os.path.isfile(input_config):
if options.verbose:
print_to_stderr("Loading in options from config file: %s \n" %
input_config)
# Command line overrides loading ~/.impalarc
configs_to_load[1] = input_config
else:
print_to_stderr('%s not found.\n' % input_config)
sys.exit(1)
Now it's fairly easy to tell that the "else" means file not found :)
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py@1656
PS1, Line 1656:
nit: formatting is a bit odd. In Python, we usually indent parser.option_list
to be in the same col as config_file.
s_options, q_options = get_config_from_file(config_file,
parser.option_list)
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py
File shell/option_parser.py:
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py@260
PS1, Line 260: help=SUPPRESS_HELP
> The global config option is only used for testing purposes, so users should
One potential use case that maybe useful is is to specify a different location
of global config than /etc/impalarc. And currently there's no way to do this
with this CR other than using --global_config that's supposed to be a hidden
flag. Furthermore, having --global_config as an impala shell is kinda redundant
especially since we already have --config_file.
I think a better way is to use an environment variable, e.g.
IMPALA_CONFIG_FILE. With this, if we want to use a different global config file
than the default /etc/impalarc, users can specify the IMPALA_CONFIG_FILE in
/etc/profile or something of that sort. If IMPALA_CONFIG_FILE is not present,
it will simply use the /etc/impalarc. This has benefits of being able to
specify a different global config file as well as making testing easier.
What do you think?
--
To view, visit http://gerrit.cloudera.org:8080/13313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816
Gerrit-Change-Number: 13313
Gerrit-PatchSet: 3
Gerrit-Owner: Ethan Xue
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Ethan Xue
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Thu, 16 May 2019 00:32:51 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 3: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/3203/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 3 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 13 May 2019 22:13:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 3: > We can include information about where the configs are being sourced upon > starting the impala shell, which is currently what we do with the user > provided impalarc. What are your thoughts on this solution? Sounds reasonable to me, thanks! -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 3 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 13 May 2019 21:33:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 2: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/3200/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 2 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 13 May 2019 21:32:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Hello Fredy Wijaya, Todd Lipcon, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13313 to look at the new patch set (#3). Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- M shell/impala_shell.py M shell/impala_shell_config_defaults.py M shell/option_parser.py A tests/shell/good_impalarc2 M tests/shell/test_shell_commandline.py 5 files changed, 76 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/3 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 3 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. (
http://gerrit.cloudera.org:8080/13313 )
Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py
File shell/impala_shell.py:
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py@1628
PS1, Line 1628: test_global_config = os.path.expanduser(options.global_config)
: if os.path.isfile(test_global_config) and test_global_config
!= default_global_config:
: default_global_config = test_global_config
: elif test_global_config != default_global_config:
: print_to_stderr('%s not found.\n' % test_global_config)
: sys.exit(1)
:
: # use path to custom file specified by user in config_file
option
: input_config = os.path.expanduser(options.config_file)
: # verify input_config, if found
: if os.path.isfile(input_config) and input_config !=
default_user_config:
: if options.verbose:
: print_to_stderr("Loading in options from config file: %s
\n" % input_config)
: # command line overrides loading ~/.impalarc
: default_user_config = input_config
: elif input_config != default_user_config:
: print_to_stderr('%s not found.\n' % input_config)
: sys.exit(1)
:
: configs_to_lo
> The logic here is difficult to follow. I believe this is basically what we
Thanks for the heads up. I tried my best to make it more readable. It does look
somewhat more complex then it should due to the prints and error checking.
--
To view, visit http://gerrit.cloudera.org:8080/13313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816
Gerrit-Change-Number: 13313
Gerrit-PatchSet: 2
Gerrit-Owner: Ethan Xue
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Ethan Xue
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Mon, 13 May 2019 20:44:19 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 2: > One question about the usability of this: it seems the most common > use case for impala-shell command line options is to specify > security-related configs (--ssl, --kerberos, etc) as well as a host > to connect to. If we put those in a global config file, we get a > usability boost that 'impala-shell' by a user will directly connect > to the right place. > > However, if the user wants to connect to a remote cluster with > different security options (eg non-Kerberos or non-SSL) will it be > confusing? For example, if I do 'impala-shell -i > some-remote-cluster.example.com', > I'll end up picking up the local cluster's Kerberos and SSL > configurations. > > This might even be considered an incompatible change, if management > tools start putting these configs in /etc/impalarc by default, and > then scripts trying to connect to non-default locations change > behavior. > > Does the impala config file format support scoping of settings by > host (like .ssh/config does?) Is that something we should consider? > If not, perhaps a compromise is to make sure connection errors > include some information about where configs are being sourced > from. Thanks for the feedback Todd. I believe the impala config file does not support scoping of settings by host, and Bikram thinks that the compromise would be a better option because of this. We can include information about where the configs are being sourced upon starting the impala shell, which is currently what we do with the user provided impalarc. What are your thoughts on this solution? -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 2 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 13 May 2019 20:50:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Hello Fredy Wijaya, Todd Lipcon, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13313 to look at the new patch set (#2). Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- M shell/impala_shell.py M shell/impala_shell_config_defaults.py M shell/option_parser.py A tests/shell/good_impalarc2 M tests/shell/test_shell_commandline.py 5 files changed, 72 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/2 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 2 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py File shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py@260 PS1, Line 260: help=SUPPRESS_HELP > why do we suppress help here? The global config option is only used for testing purposes, so users should not be able to see it in the help text or provide their own file. Normally, /etc/impalarc should always be picked up but to make it easier for testing, we enable a custom global impalarc to be supplied. Another example of a "hidden" shell option used for testing is in start-impala-cluster.py. -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 1 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 13 May 2019 18:31:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has posted comments on this change. (
http://gerrit.cloudera.org:8080/13313 )
Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config
..
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py
File shell/impala_shell.py:
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py@1612
PS1, Line 1612: h
> flake8: E501 line too long (92 > 90 characters)
Done
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py@1615
PS1, Line 1615: l
> flake8: E501 line too long (92 > 90 characters)
Done
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py@1624
PS1, Line 1624: user_config = impala_shell_defaults.get("config_file")
: global_config = impala_shell_defaults.get("global_config")
> we should call these variables.
Done
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell_config_defaults.py
File shell/impala_shell_config_defaults.py:
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell_config_defaults.py@55
PS1, Line 55: .impalarc
> Usually files in /etc are not dot files. So /etc/impalarc is better.
Done
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py
File shell/option_parser.py:
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py@289
PS1, Line 289: n
> flake8: E714 test for object identity should be 'is not'
Done
--
To view, visit http://gerrit.cloudera.org:8080/13313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816
Gerrit-Change-Number: 13313
Gerrit-PatchSet: 1
Gerrit-Owner: Ethan Xue
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Ethan Xue
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Mon, 13 May 2019 18:25:52 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 1: One question about the usability of this: it seems the most common use case for impala-shell command line options is to specify security-related configs (--ssl, --kerberos, etc) as well as a host to connect to. If we put those in a global config file, we get a usability boost that 'impala-shell' by a user will directly connect to the right place. However, if the user wants to connect to a remote cluster with different security options (eg non-Kerberos or non-SSL) will it be confusing? For example, if I do 'impala-shell -i some-remote-cluster.example.com', I'll end up picking up the local cluster's Kerberos and SSL configurations. This might even be considered an incompatible change, if management tools start putting these configs in /etc/impalarc by default, and then scripts trying to connect to non-default locations change behavior. Does the impala config file format support scoping of settings by host (like .ssh/config does?) Is that something we should consider? If not, perhaps a compromise is to make sure connection errors include some information about where configs are being sourced from. -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 1 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 13 May 2019 17:17:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/13313 )
Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py
File shell/impala_shell.py:
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py@1624
PS1, Line 1624: user_config = impala_shell_defaults.get("config_file")
: global_config = impala_shell_defaults.get("global_config")
we should call these variables.
default_user_config and default_global_config
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py@1628
PS1, Line 1628: # use path to custom file specified by user in config_file
option
: input_config = os.path.expanduser(options.config_file)
: # for testing: use path to custom global config
: test_global_config = os.path.expanduser(options.global_config)
:
: if os.path.isfile(test_global_config) and test_global_config
!= global_config:
: configs_to_load[0] = test_global_config
: elif test_global_config != global_config:
: print_to_stderr('%s not found.\n' % test_global_config)
: sys.exit(1)
:
: # verify input_config, if found
: if os.path.isfile(input_config) and input_config !=
user_config:
: if options.verbose:
: print_to_stderr("Loading in options from config file: %s
\n" % input_config)
: # Command line overrides loading ~/.impalarc
: configs_to_load[1] = input_config
: elif input_config != user_config:
: print_to_stderr('%s not found.\n' % input_config)
: sys.exit(1)
The logic here is difficult to follow. I believe this is basically what we want:
if input config is specified: check if the input file exists then use it
else: use the one from the default
I think we should try to simplify the logic a bit.
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell_config_defaults.py
File shell/impala_shell_config_defaults.py:
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell_config_defaults.py@55
PS1, Line 55: .impalarc
Usually files in /etc are not dot files. So /etc/impalarc is better.
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py
File shell/option_parser.py:
http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py@260
PS1, Line 260: help=SUPPRESS_HELP
why do we suppress help here?
--
To view, visit http://gerrit.cloudera.org:8080/13313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816
Gerrit-Change-Number: 13313
Gerrit-PatchSet: 1
Gerrit-Owner: Ethan Xue
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Sat, 11 May 2019 00:43:28 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 1: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/3187/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 1 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 11 May 2019 00:32:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13313 ) Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py@1612 PS1, Line 1612: h flake8: E501 line too long (92 > 90 characters) http://gerrit.cloudera.org:8080/#/c/13313/1/shell/impala_shell.py@1615 PS1, Line 1615: l flake8: E501 line too long (92 > 90 characters) http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py File shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/13313/1/shell/option_parser.py@289 PS1, Line 289: n flake8: E714 test for object identity should be 'is not' -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 1 Gerrit-Owner: Ethan Xue Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 11 May 2019 00:02:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6042: Allow Impala shell to use a global impalarc config
Ethan Xue has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13313 Change subject: IMPALA-6042: Allow Impala shell to use a global impalarc config .. IMPALA-6042: Allow Impala shell to use a global impalarc config Currently, impalarc files can be specified on a per-user basis (stored in ~/.impalarc), and they aren't created by default. The Impala shell should pick up /etc/impalarc as well, in addition to the user-specific configurations. The intent here is to allow a "global" configuration of the shell by a system administrator. Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 --- M shell/impala_shell.py M shell/impala_shell_config_defaults.py M shell/option_parser.py A tests/shell/good_impalarc2 M tests/shell/test_shell_commandline.py 5 files changed, 70 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/13313/1 -- To view, visit http://gerrit.cloudera.org:8080/13313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3a3179b6d9c9e3b2b01d6d3c5847cadb68782816 Gerrit-Change-Number: 13313 Gerrit-PatchSet: 1 Gerrit-Owner: Ethan Xue
