You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Steve Carlin (Code Review)" <ge...@cloudera.org> on 2021/07/07 22:57:15 UTC

[Impala-ASF-CR] IMPALA-10778:

Steve Carlin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17660


Change subject: IMPALA-10778:
......................................................................

IMPALA-10778:

Impala-shell already uses HS2 protocol to connect to Impalad. 
This commit allows impala-shell to connect to any server using
the hs2 protocol. This will be done via the "--strict" option.

When the "--strict" option is turned on, only features supported
by hs2 will work. For instance, "runtime-profile" is an impalad
specific feature and will be disabled.

Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
---
M fe/src/test/resources/hive-site.xml.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M testdata/bin/run-hive-server.sh
M tests/common/impala_test_suite.py
M tests/common/test_dimensions.py
M tests/conftest.py
M tests/shell/test_shell_client.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
13 files changed, 528 insertions(+), 92 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17660/1
-- 
To view, visit http://gerrit.cloudera.org:8080/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 14:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7423/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 14
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Aug 2021 13:19:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 12:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7406/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 12
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 20 Aug 2021 02:18:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17660

to look at the new patch set (#7).

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................

IMPALA-10778: Allow impala-shell to connect directly to HS2

Impala-shell already uses HS2 protocol to connect to Impalad.
This commit allows impala-shell to connect to any server (for
example, Hive) using the hs2 protocol. This will be done via
the "--strict_hs2_protocol" option.

When the "--strict_hs2_protocol" option is turned on, only features
supported by hs2 will work. For instance, "runtime-profile" is an
impalad specific feature and will be disabled.

The "--strict_hs2_protocol" will only work on servers that abide
by the strict definition of what is supported by HS2. So one will
be able to connect to Hive in this mode, but connections to Impala
will not work. Any feature supported by Hive (e.g. kerberos
authentication) should work as well.

Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
---
M fe/src/test/resources/hive-site.xml.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/common/impala_test_suite.py
M tests/common/test_dimensions.py
M tests/conftest.py
M tests/shell/test_shell_client.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
12 files changed, 322 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17660/7
-- 
To view, visit http://gerrit.cloudera.org:8080/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 14: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 14
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Aug 2021 11:18:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 11:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7399/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 11
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Aug 2021 18:31:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 11: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7399/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 11
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 18 Aug 2021 04:32:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 14: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7413/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 14
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Aug 2021 18:25:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 3:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@9
PS3, Line 9: Impala-shell already uses HS2 protocol to connect to Impalad. 
nit: extra whitespace


http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@10
PS3, Line 10:  any server
Can you add "for example Hive"?


http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@11
PS3, Line 11: --strict
I would prefer a clearer name, like strict_hs2 or no_hs2_extensions. Strict could mean so many things.


http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@12
PS3, Line 12: 
Can you add more info about what this client is capable of in strict mode? Can it connect to Hive? Can it connect to Impala? Can it use authentication mechanisms? One issue that comes up is that we use a different cookie name in Impala than in Hive, so cookie auth won't work with Hive.


http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@13
PS3, Line 13: When the "--strict" option is turned on
Wouldn't it be possible to detect this somehow at runtime? For example by calling "select version()" first.


http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_client.py@1105
PS3, Line 1105:     return ""
Shouldn't we assert when calling unsupported functions? Will this be actually used?


http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_shell.py@191
PS3, Line 191: test_mode
Do we use test mode for anything else than setting ldap_password? if not, then why not simply pass the password?


http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_shell.py@574
PS3, Line 574: 'steve', 'password
Will this work on other machines?


http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/test_shell_commandline.py@142
PS3, Line 142:     if vector.get_value('strict_protocol') and vector.get_value('protocol') == 'beeswax':
             :       pytest.skip("Beeswax protocol not supported in strict mode.")
you can add this logic as a skipIf annotation in https://github.com/apache/impala/blob/master/tests/common/skip.py



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jul 2021 17:38:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_interactive.py@352
PS5, Line 352:       pytest.skip("Failed, need to investigate.")
> Please file a JIRA and reference here if you want to leave this TODO.
Done


http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_interactive.py@1092
PS5, Line 1092:       pytest.skip("The now() function is not supported in strict hs2 mode.")
> Heh, yeah, this is more of a frontend issue
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 26 Jul 2021 23:57:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 10:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7398/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 10
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Aug 2021 07:24:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/17660/1/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/17660/1/shell/impala_client.py@1072
PS1, Line 1072: class StrictHS2Client(ImpalaHS2Client):
> flake8: E302 expected 2 blank lines, found 1
Done


http://gerrit.cloudera.org:8080/#/c/17660/1/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/17660/1/shell/impala_shell.py@914
PS1, Line 914: \
> flake8: E502 the backslash is redundant between brackets
Done


http://gerrit.cloudera.org:8080/#/c/17660/1/tests/common/test_dimensions.py
File tests/common/test_dimensions.py:

http://gerrit.cloudera.org:8080/#/c/17660/1/tests/common/test_dimensions.py@131
PS1, Line 131: def create_client_protocol_strict_dimension():
> flake8: E302 expected 2 blank lines, found 1
Done


http://gerrit.cloudera.org:8080/#/c/17660/1/tests/common/test_dimensions.py@134
PS1, Line 134: def create_client_protocol_no_strict_dimension():
> flake8: E302 expected 2 blank lines, found 1
Done


http://gerrit.cloudera.org:8080/#/c/17660/1/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/17660/1/tests/shell/test_shell_commandline.py@277
PS1, Line 277:  
> flake8: E241 multiple spaces after ','
Done


http://gerrit.cloudera.org:8080/#/c/17660/1/tests/shell/test_shell_commandline.py@615
PS1, Line 615:  
> line has trailing whitespace
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 23:07:23 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 5:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/17660/5/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/17660/5/shell/impala_client.py@1102
PS5, Line 1102:     # CloseImpalaOperation rpc is idempotent for non dml queries and so safe to retry.
Comment doesn't match retry flag.


http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_commandline.py@360
PS5, Line 360:     if vector.get_value('strict_hs2_protocol'):
nit: this could be in a function to avoid duplication.


http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_interactive.py@352
PS5, Line 352:       pytest.skip("Failed, need to investigate.")
Please file a JIRA and reference here if you want to leave this TODO.


http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_interactive.py@518
PS5, Line 518:       pytest.skip("Not working in strict hs2 mode.")
Another TODO JIRA? Several more below.


http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_interactive.py@1092
PS5, Line 1092:       pytest.skip("The now() function is not supported in strict hs2 mode.")
Is this an impala-shell limitation or a frontend limitation?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Jul 2021 17:57:39 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778:

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778:
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/17660/1/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/17660/1/shell/impala_client.py@1072
PS1, Line 1072: class StrictHS2Client(ImpalaHS2Client):
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/17660/1/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/17660/1/shell/impala_shell.py@914
PS1, Line 914: \
flake8: E502 the backslash is redundant between brackets


http://gerrit.cloudera.org:8080/#/c/17660/1/tests/common/test_dimensions.py
File tests/common/test_dimensions.py:

http://gerrit.cloudera.org:8080/#/c/17660/1/tests/common/test_dimensions.py@131
PS1, Line 131: def create_client_protocol_strict_dimension():
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/17660/1/tests/common/test_dimensions.py@134
PS1, Line 134: def create_client_protocol_no_strict_dimension():
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/17660/1/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/17660/1/tests/shell/test_shell_commandline.py@277
PS1, Line 277:  
flake8: E241 multiple spaces after ','


http://gerrit.cloudera.org:8080/#/c/17660/1/tests/shell/test_shell_commandline.py@615
PS1, Line 615:  
flake8: W293 blank line contains whitespace


http://gerrit.cloudera.org:8080/#/c/17660/1/tests/shell/test_shell_commandline.py@615
PS1, Line 615:  
line has trailing whitespace



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 22:58:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9178/ : 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/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Jul 2021 00:18:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 11:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7403/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 11
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 18 Aug 2021 13:57:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 7:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/17660/7/shell/impala_shell.py@192
PS7, Line 192:       self.ldap_password = 'password'
can you add a comment about the reason for using a dummy password?


http://gerrit.cloudera.org:8080/#/c/17660/7/shell/impala_shell.py@574
PS7, Line 574:         return StrictHS2Client(self.impalad, self.fetch_size, self.kerberos_host_fqdn,
             :                           self.use_kerberos, self.kerberos_service_name, self.use_ssl,
             :                           self.ca_cert, self.user, self.ldap_password, True,
             :                           self.client_connect_timeout_ms, self.verbose,
             :                           use_http_base_transport=False, http_path=self.http_path,
             :                           http_cookie_names=None)
optional: I think that using a member like is_strict_mode could be a better option than having a subclass - my concern is that someone may change ImpalaHS2Client but forget to update StrictHS2Client. E.g. adding a new parameter to the constructor, but not adding it to line 574/581. What do you think?


http://gerrit.cloudera.org:8080/#/c/17660/7/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/17660/7/shell/option_parser.py@283
PS7, Line 283:                          "Only useful if connecting straight to hs2 instead of Impala.")
Can you mention that 10001 is the default port in this case? (btw, what is the reason for 10001?)


http://gerrit.cloudera.org:8080/#/c/17660/7/shell/option_parser.py@284
PS7, Line 284: test_mode
I would prefer a more exact name like --use_ldap_test_password. We use impala shell in many unit tests without using this flag.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Aug 2021 19:13:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17660/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17660/8//COMMIT_MSG@22
PS8, Line 22: authentication) should work as well.
> Sorry, took awhile for me to get back.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 16 Aug 2021 16:53:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/17660/5/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/17660/5/shell/impala_client.py@1102
PS5, Line 1102:     # CloseImpalaOperation rpc is idempotent for non dml queries and so safe to retry.
> Comment doesn't match retry flag.
Done


http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_commandline.py@360
PS5, Line 360:     if vector.get_value('strict_hs2_protocol'):
> nit: this could be in a function to avoid duplication.
I'm not sure what you're looking for here...

Are you looking for a one line function that says do_not_run_if_strict_protocol("Here is My error message")
?

Not sure that adds much aesthetically since it only removes one line.


http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_interactive.py@352
PS5, Line 352:       pytest.skip("Failed, need to investigate.")
> Please file a JIRA and reference here if you want to leave this TODO.
Makes sense, but should I just make one umbrella Jira for all the TODOs?  Or file each individually (which could be a lot)?


http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_interactive.py@1092
PS5, Line 1092:       pytest.skip("The now() function is not supported in strict hs2 mode.")
> Is this an impala-shell limitation or a frontend limitation?
Heh, yeah, this is more of a frontend issue



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Jul 2021 22:16:08 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17660

to look at the new patch set (#4).

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................

IMPALA-10778: Allow impala-shell to connect directly to HS2

Impala-shell already uses HS2 protocol to connect to Impalad.
This commit allows impala-shell to connect to any server (for
example, Hive) using the hs2 protocol. This will be done via
the "--strict_hs2_protocol" option.

When the "--strict_hs2_protocol" option is turned on, only features
supported by hs2 will work. For instance, "runtime-profile" is an
impalad specific feature and will be disabled.

The "--strict_hs2_protocol" will only work on servers that abide
by the strict definition of what is supported by HS2. So one will
be able to connect to Hive in this mode, but connections to Impala
will not work. Any feature supported by Hive (e.g. kerberos
authentication) should work as well.

Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
---
M fe/src/test/resources/hive-site.xml.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/common/impala_test_suite.py
M tests/common/test_dimensions.py
M tests/conftest.py
M tests/shell/test_shell_client.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
12 files changed, 323 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17660/4
-- 
To view, visit http://gerrit.cloudera.org:8080/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9129/ : 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/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 20 Jul 2021 21:27:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 7:

I have no further comments but Csaba should review.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 30 Jul 2021 01:55:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 14: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 14
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Aug 2021 19:36:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17660

to look at the new patch set (#6).

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................

IMPALA-10778: Allow impala-shell to connect directly to HS2

Impala-shell already uses HS2 protocol to connect to Impalad.
This commit allows impala-shell to connect to any server (for
example, Hive) using the hs2 protocol. This will be done via
the "--strict_hs2_protocol" option.

When the "--strict_hs2_protocol" option is turned on, only features
supported by hs2 will work. For instance, "runtime-profile" is an
impalad specific feature and will be disabled.

The "--strict_hs2_protocol" will only work on servers that abide
by the strict definition of what is supported by HS2. So one will
be able to connect to Hive in this mode, but connections to Impala
will not work. Any feature supported by Hive (e.g. kerberos
authentication) should work as well.

Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
---
M fe/src/test/resources/hive-site.xml.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/common/impala_test_suite.py
M tests/common/test_dimensions.py
M tests/conftest.py
M tests/shell/test_shell_client.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
12 files changed, 321 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17660/6
-- 
To view, visit http://gerrit.cloudera.org:8080/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 6
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17660

to look at the new patch set (#5).

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................

IMPALA-10778: Allow impala-shell to connect directly to HS2

Impala-shell already uses HS2 protocol to connect to Impalad.
This commit allows impala-shell to connect to any server (for
example, Hive) using the hs2 protocol. This will be done via
the "--strict_hs2_protocol" option.

When the "--strict_hs2_protocol" option is turned on, only features
supported by hs2 will work. For instance, "runtime-profile" is an
impalad specific feature and will be disabled.

The "--strict_hs2_protocol" will only work on servers that abide
by the strict definition of what is supported by HS2. So one will
be able to connect to Hive in this mode, but connections to Impala
will not work. Any feature supported by Hive (e.g. kerberos
authentication) should work as well.

Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
---
M fe/src/test/resources/hive-site.xml.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/common/impala_test_suite.py
M tests/common/test_dimensions.py
M tests/conftest.py
M tests/shell/test_shell_client.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
12 files changed, 322 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17660/5
-- 
To view, visit http://gerrit.cloudera.org:8080/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 10: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 10
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Aug 2021 07:24:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17660

to look at the new patch set (#12).

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................

IMPALA-10778: Allow impala-shell to connect directly to HS2

Impala-shell already uses HS2 protocol to connect to Impalad.
This commit allows impala-shell to connect to any server (for
example, Hive) using the hs2 protocol. This will be done via
the "--strict_hs2_protocol" option.

When the "--strict_hs2_protocol" option is turned on, only features
supported by hs2 will work. For instance, "runtime-profile" is an
impalad specific feature and will be disabled.

The "--strict_hs2_protocol" will only work on servers that abide
by the strict definition of what is supported by HS2. So one will
be able to connect to Hive in this mode, but connections to Impala
will not work. Any feature supported by Hive (e.g. kerberos
authentication) should work as well.

Note: While authentication should work, the test framework is not
set up to create an HS2 server that does authentication at this point
so this feature should be used with caution.
Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
---
M fe/src/test/resources/hive-site.xml.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/common/impala_test_suite.py
M tests/common/test_dimensions.py
M tests/common/test_vector.py
M tests/conftest.py
M tests/shell/test_shell_client.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
13 files changed, 334 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17660/12
-- 
To view, visit http://gerrit.cloudera.org:8080/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 12
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 13: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 13
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 20 Aug 2021 21:16:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9047/ : 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/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 23:19:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 11:

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7403/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 11
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 18 Aug 2021 23:58:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17660/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17660/8//COMMIT_MSG@22
PS8, Line 22: authentication) should work as well.
> Can you add that secure connections to Hive are not tested? IMO this also m
Sorry, took awhile for me to get back.

Actually, it should be test.  For instance, test_ldap_password_from_shell isn't skipped for this, so regular ldap tests should be going through the strict protocols.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Aug 2021 03:25:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 3:

(14 comments)

http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@9
PS3, Line 9: Impala-shell already uses HS2 protocol to connect to Impalad. 
> nit: extra whitespace
Done


http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@10
PS3, Line 10:  any server
> Can you add "for example Hive"?
Done


http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@11
PS3, Line 11: --strict
> I would prefer a clearer name, like strict_hs2 or no_hs2_extensions. Strict
Done


http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@12
PS3, Line 12: 
> Can you add more info about what this client is capable of in strict mode? 
Done


http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@13
PS3, Line 13: When the "--strict" option is turned on
> Wouldn't it be possible to detect this somehow at runtime? For example by c
I don't think this is gonna be possible.

The default mode in Impala creates a vanilla TBufferedTransport object and passes it into thrift. I tried this mode on HiveServer2 and it did not work.  So the only way to really test the default is to experiment on connection, but I don't think that's a good idea since it would fail to connect the first time, and that would affect the number of attempts tried for a server.


http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_client.py@1091
PS3, Line 1091:     resp = self._do_hs2_rpc(CloseOperation, retry_on_error=True)
> Not sure if we should retry here. The only case that is worth retrying is i
Done


http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_client.py@1105
PS3, Line 1105:     return ""
> Shouldn't we assert when calling unsupported functions? Will this be actual
This is called from impala_shell as a virtual method, so I can't assert here.


http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_shell.py@191
PS3, Line 191: test_mode
> Do we use test mode for anything else than setting ldap_password? if not, t
Ok, so I thought about this...

The problem I ran into is that the adding of the ldap_password was a bit awkward.  If we pass in an ldap_password, impala-shell thinks we're in ldap mode.  So if we wanted to be in kerberos mode, it doesn't really work to pass in the ldap password.

The logic would have to be that if we are in strict mode and not in kerberos mode, send in the password for all calls to impala-shell.

The logic seemed a bit too awkward since the kerberos decision is made within the top level file (e.g. test_shell_commandline) whereas the ldap_password decision would either have to be in every test method (ick) or within util.py when the command is generated, and that logic seemed a bit icky to me.

By passing this "--test_mode", I think things came out the cleanest.

But I'm willing to change if you think there's another way.


http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_shell.py@574
PS3, Line 574: 'steve', 'password
> Will this work on other machines?
Whoops!


http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_shell_config_defaults.py
File shell/impala_shell_config_defaults.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_shell_config_defaults.py@58
PS3, Line 58:             'strict_protocol': False,
> strict_hs2_protocol
Done


http://gerrit.cloudera.org:8080/#/c/17660/3/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/shell/option_parser.py@280
PS3, Line 280:   parser.add_option("--strict_protocol", dest="strict_protocol", action="store_true",
> strict_hs2_protocol. Make it clear this only applies to HS2.
Done


http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/test_shell_commandline.py@142
PS3, Line 142:     if vector.get_value('strict_protocol') and vector.get_value('protocol') == 'beeswax':
             :       pytest.skip("Beeswax protocol not supported in strict mode.")
> you can add this logic as a skipIf annotation in https://github.com/apache/
I figured out I can get rid of this via the "add_constraint" function


http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/test_shell_interactive.py@193
PS3, Line 193:     if vector.get_value('strict_protocol') and vector.get_value('protocol') == 'beeswax':
> May be cleaner to split the tests into 2 vectors.
I think I figured out a way to get rid of this.

By calling add_constraint when adding dimensions, I can test for this specific condition and always skip it.


http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/util.py
File tests/shell/util.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/util.py@318
PS3, Line 318:     f = open('/home/steve/tmp/sjc24.txt', 'w')
> change hard-coded path.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 20 Jul 2021 21:00:43 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9263/ : 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/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Aug 2021 21:42:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 13:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7409/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 13
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 20 Aug 2021 15:07:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 8: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17660/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17660/8//COMMIT_MSG@22
PS8, Line 22: authentication) should work as well.
Can you add that secure connections to Hive are not tested? IMO this also means that Impala shell + Hive is mainly for testing at the moment and not a production level client.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 15:32:54 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 11: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 11
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Aug 2021 18:31:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 14:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7413/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 14
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Aug 2021 11:18:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 9: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 9
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 16 Aug 2021 18:31:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17660

to look at the new patch set (#2).

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................

IMPALA-10778: Allow impala-shell to connect directly to HS2

Impala-shell already uses HS2 protocol to connect to Impalad. 
This commit allows impala-shell to connect to any server using
the hs2 protocol. This will be done via the "--strict" option.

When the "--strict" option is turned on, only features supported
by hs2 will work. For instance, "runtime-profile" is an impalad
specific feature and will be disabled.

Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
---
M fe/src/test/resources/hive-site.xml.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M testdata/bin/run-hive-server.sh
M tests/common/impala_test_suite.py
M tests/common/test_dimensions.py
M tests/conftest.py
M tests/shell/test_shell_client.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
13 files changed, 531 insertions(+), 91 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17660/2
-- 
To view, visit http://gerrit.cloudera.org:8080/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 13:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9338/ : 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/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 13
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 20 Aug 2021 14:50:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17660

to look at the new patch set (#9).

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................

IMPALA-10778: Allow impala-shell to connect directly to HS2

Impala-shell already uses HS2 protocol to connect to Impalad.
This commit allows impala-shell to connect to any server (for
example, Hive) using the hs2 protocol. This will be done via
the "--strict_hs2_protocol" option.

When the "--strict_hs2_protocol" option is turned on, only features
supported by hs2 will work. For instance, "runtime-profile" is an
impalad specific feature and will be disabled.

The "--strict_hs2_protocol" will only work on servers that abide
by the strict definition of what is supported by HS2. So one will
be able to connect to Hive in this mode, but connections to Impala
will not work. Any feature supported by Hive (e.g. kerberos
authentication) should work as well.

Note: While authentication should work, the test framework is not
set up to create an HS2 server that does authentication at this point
so this feature should be used with caution.
Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
---
M fe/src/test/resources/hive-site.xml.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/common/impala_test_suite.py
M tests/common/test_dimensions.py
M tests/conftest.py
M tests/shell/test_shell_client.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
12 files changed, 328 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17660/9
-- 
To view, visit http://gerrit.cloudera.org:8080/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 9
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9130/ : 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/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 20 Jul 2021 22:20:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17660

to look at the new patch set (#3).

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................

IMPALA-10778: Allow impala-shell to connect directly to HS2

Impala-shell already uses HS2 protocol to connect to Impalad. 
This commit allows impala-shell to connect to any server using
the hs2 protocol. This will be done via the "--strict" option.

When the "--strict" option is turned on, only features supported
by hs2 will work. For instance, "runtime-profile" is an impalad
specific feature and will be disabled.

Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
---
M fe/src/test/resources/hive-site.xml.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/common/impala_test_suite.py
M tests/common/test_dimensions.py
M tests/conftest.py
M tests/shell/test_shell_client.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
12 files changed, 530 insertions(+), 86 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17660/3
-- 
To view, visit http://gerrit.cloudera.org:8080/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17660

to look at the new patch set (#13).

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................

IMPALA-10778: Allow impala-shell to connect directly to HS2

Impala-shell already uses HS2 protocol to connect to Impalad.
This commit allows impala-shell to connect to any server (for
example, Hive) using the hs2 protocol. This will be done via
the "--strict_hs2_protocol" option.

When the "--strict_hs2_protocol" option is turned on, only features
supported by hs2 will work. For instance, "runtime-profile" is an
impalad specific feature and will be disabled.

The "--strict_hs2_protocol" will only work on servers that abide
by the strict definition of what is supported by HS2. So one will
be able to connect to Hive in this mode, but connections to Impala
will not work. Any feature supported by Hive (e.g. kerberos
authentication) should work as well.

Note: While authentication should work, the test framework is not
set up to create an HS2 server that does authentication at this point
so this feature should be used with caution.
Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
---
M fe/src/test/resources/hive-site.xml.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/common/impala_test_suite.py
M tests/common/test_dimensions.py
M tests/common/test_vector.py
M tests/conftest.py
M tests/shell/test_shell_client.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
13 files changed, 334 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17660/13
-- 
To view, visit http://gerrit.cloudera.org:8080/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 13
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................

IMPALA-10778: Allow impala-shell to connect directly to HS2

Impala-shell already uses HS2 protocol to connect to Impalad.
This commit allows impala-shell to connect to any server (for
example, Hive) using the hs2 protocol. This will be done via
the "--strict_hs2_protocol" option.

When the "--strict_hs2_protocol" option is turned on, only features
supported by hs2 will work. For instance, "runtime-profile" is an
impalad specific feature and will be disabled.

The "--strict_hs2_protocol" will only work on servers that abide
by the strict definition of what is supported by HS2. So one will
be able to connect to Hive in this mode, but connections to Impala
will not work. Any feature supported by Hive (e.g. kerberos
authentication) should work as well.

Note: While authentication should work, the test framework is not
set up to create an HS2 server that does authentication at this point
so this feature should be used with caution.
Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Reviewed-on: http://gerrit.cloudera.org:8080/17660
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/test/resources/hive-site.xml.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/common/impala_test_suite.py
M tests/common/test_dimensions.py
M tests/common/test_vector.py
M tests/conftest.py
M tests/shell/test_shell_client.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
13 files changed, 334 insertions(+), 81 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 15
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 7:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/17660/7/shell/impala_shell.py@192
PS7, Line 192:       self.ldap_password = 'password'
> can you add a comment about the reason for using a dummy password?
Done


http://gerrit.cloudera.org:8080/#/c/17660/7/shell/impala_shell.py@574
PS7, Line 574:         return StrictHS2Client(self.impalad, self.fetch_size, self.kerberos_host_fqdn,
             :                           self.use_kerberos, self.kerberos_service_name, self.use_ssl,
             :                           self.ca_cert, self.user, self.ldap_password, True,
             :                           self.client_connect_timeout_ms, self.verbose,
             :                           use_http_base_transport=False, http_path=self.http_path,
             :                           http_cookie_names=None)
> optional: I think that using a member like is_strict_mode could be a better
I don't know...

I hear what you're saying.  But if someone is adding a parameter, I would hope that they notice the other flavor here and realize it is needed?  All the calls to the constructor are right here.

I really don't like adding the "if" clause, since I think it dirties up the code.


http://gerrit.cloudera.org:8080/#/c/17660/7/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/17660/7/shell/option_parser.py@283
PS7, Line 283:                          "Only useful if connecting straight to hs2 instead of Impala.")
> Can you mention that 10001 is the default port in this case? (btw, what is 
Done


http://gerrit.cloudera.org:8080/#/c/17660/7/shell/option_parser.py@284
PS7, Line 284: test_mode
> I would prefer a more exact name like --use_ldap_test_password. We use impa
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Aug 2021 21:21:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 4:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/17660/4/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/17660/4/shell/impala_shell.py@193
PS4, Line 193: s
flake8: E501 line too long (92 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/17660/4/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/17660/4/tests/shell/test_shell_commandline.py@141
PS4, Line 141:  
flake8: W291 trailing whitespace


http://gerrit.cloudera.org:8080/#/c/17660/4/tests/shell/test_shell_commandline.py@141
PS4, Line 141:           v.get_value('protocol') != 'beeswax' or 
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/17660/4/tests/shell/test_shell_commandline.py@142
PS4, Line 142: n
flake8: E126 continuation line over-indented for hanging indent


http://gerrit.cloudera.org:8080/#/c/17660/4/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/17660/4/tests/shell/test_shell_interactive.py@172
PS4, Line 172:  
flake8: W291 trailing whitespace


http://gerrit.cloudera.org:8080/#/c/17660/4/tests/shell/test_shell_interactive.py@172
PS4, Line 172:           v.get_value('protocol') != 'beeswax' or 
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/17660/4/tests/shell/test_shell_interactive.py@173
PS4, Line 173: n
flake8: E126 continuation line over-indented for hanging indent



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 20 Jul 2021 21:01:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 10: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7398/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 10
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Aug 2021 17:24:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9147/ : 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/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 6
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Jul 2021 22:37:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 12: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7405/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 12
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 20 Aug 2021 01:58:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 12:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7405/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 12
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 19 Aug 2021 15:58:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 12:

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7406/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 12
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 20 Aug 2021 12:18:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_client.py@1091
PS3, Line 1091:     resp = self._do_hs2_rpc(CloseOperation, retry_on_error=True)
Not sure if we should retry here. The only case that is worth retrying is if there was a transport error but the session is still alive. I realize this is copied but the backend may have different error states.


http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_shell_config_defaults.py
File shell/impala_shell_config_defaults.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_shell_config_defaults.py@58
PS3, Line 58:             'strict_protocol': False,
strict_hs2_protocol


http://gerrit.cloudera.org:8080/#/c/17660/3/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/shell/option_parser.py@280
PS3, Line 280:   parser.add_option("--strict_protocol", dest="strict_protocol", action="store_true",
strict_hs2_protocol. Make it clear this only applies to HS2.


http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/test_shell_interactive.py@193
PS3, Line 193:     if vector.get_value('strict_protocol') and vector.get_value('protocol') == 'beeswax':
May be cleaner to split the tests into 2 vectors.


http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/util.py
File tests/shell/util.py:

http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/util.py@318
PS3, Line 318:     f = open('/home/steve/tmp/sjc24.txt', 'w')
change hard-coded path.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Jul 2021 17:44:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9048/ : 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/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 23:27:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 12:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9327/ : 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/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 12
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 19 Aug 2021 16:18:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9049/ : 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/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 23:47:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17660

to look at the new patch set (#8).

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................

IMPALA-10778: Allow impala-shell to connect directly to HS2

Impala-shell already uses HS2 protocol to connect to Impalad.
This commit allows impala-shell to connect to any server (for
example, Hive) using the hs2 protocol. This will be done via
the "--strict_hs2_protocol" option.

When the "--strict_hs2_protocol" option is turned on, only features
supported by hs2 will work. For instance, "runtime-profile" is an
impalad specific feature and will be disabled.

The "--strict_hs2_protocol" will only work on servers that abide
by the strict definition of what is supported by HS2. So one will
be able to connect to Hive in this mode, but connections to Impala
will not work. Any feature supported by Hive (e.g. kerberos
authentication) should work as well.

Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
---
M fe/src/test/resources/hive-site.xml.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/common/impala_test_suite.py
M tests/common/test_dimensions.py
M tests/conftest.py
M tests/shell/test_shell_client.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
12 files changed, 328 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17660/8
-- 
To view, visit http://gerrit.cloudera.org:8080/17660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_commandline.py@360
PS5, Line 360:     if vector.get_value('strict_hs2_protocol'):
> I'm not sure what you're looking for here...
I was thinking something like skip_if_strict_hs2() and not duplicate the strings but OK to leave as-is if you prefer.


http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_interactive.py@352
PS5, Line 352:       pytest.skip("Failed, need to investigate.")
> Makes sense, but should I just make one umbrella Jira for all the TODOs?  O
Umbrella JIRA Is fine as long as the issues are enumerated.


http://gerrit.cloudera.org:8080/#/c/17660/5/tests/shell/test_shell_interactive.py@1092
PS5, Line 1092:       pytest.skip("The now() function is not supported by hive frontend.")
> Heh, yeah, this is more of a frontend issue
string can say that test is Impala-specific.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 6
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 23 Jul 2021 16:54:39 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 13: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 13
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Aug 2021 11:16:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10778: Allow impala-shell to connect directly to HS2

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )

Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17660/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17660/8//COMMIT_MSG@22
PS8, Line 22: authentication) should work as well.
> Sorry, took awhile for me to get back.
There are some authentication related shell tests that are called from "custom cluster FE tests" like https://github.com/apache/impala/blob/master/fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java

It sets up a test ldap server in Java than runs impala-shell processes to verify that it behaves as expected. It could be potentially modified to try it with both strict and normal hs2.

test_ldap_password_from_shell only tests failed authentication, as there is no LDAP server running, see
https://github.com/apache/impala/blob/b28da054f3595bb92873433211438306fc22fbc7/tests/shell/test_shell_commandline.py#L668

But my main concern is actually different: all these tests connect to Impala, not Hive, so we cannot say that we have tested secure connections to Hive with impala-shell. So I think that we should note that these would probably work, but we didn't verify this. There can be subtle differences in Impala/Hive auth workflow (for example the name of the auth cookie as I mentioned earlier in another comment)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e
Gerrit-Change-Number: 17660
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 16 Aug 2021 15:12:38 +0000
Gerrit-HasComments: Yes