You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org> on 2021/07/09 06:39:46 UTC

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell 

Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17667


Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................

IMPALA-10784: Add support for retaining cookies in impala-shell

IMPALA-10234 added support for cookie authentication for LDAP to
impala-shell. But it does not accept user input cookie name via
startup flags, and it retains only one cookie.

In some scenarios, we could use proxy to manage the sessions with
additional HTTP cookies added by proxy.
This patch made cookie support more generic for impala-shell.
It lets the user specify cookie names via a startup flag
"--http_cookie_names" and could retain more than one cookies.

Testing:
 - Passed impala-shell unit-tests.
 - Manualy tested the multiple cookies in HTTP headers with a
   customized Impala server which could send and receive multiple
   cookies.
 - Passed core tests.

Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
---
M shell/ImpalaHttpClient.py
M shell/cookie_util.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
M tests/shell/test_cookie_util.py
6 files changed, 97 insertions(+), 64 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jul 2021 07:09:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 3:

(4 comments)

Looks good, just a few nits

http://gerrit.cloudera.org:8080/#/c/17667/3/shell/ImpalaHttpClient.py
File shell/ImpalaHttpClient.py:

http://gerrit.cloudera.org:8080/#/c/17667/3/shell/ImpalaHttpClient.py@66
PS3, Line 66: http_cookie_names is comma-separated list of cookie names which are used to specify
            :     the cookie-based authentication or session management
nit: http_cookie_names is used to specify a comma-separated list of possible cookie names used for cookie-based authentication  or session management. If a cookie with one of these names is returned in an http response by the server or an intermediate proxy then it will be included in each subsequent request for the same connection.


http://gerrit.cloudera.org:8080/#/c/17667/3/shell/ImpalaHttpClient.py@170
PS3, Line 170: updateHttpCookie
nit: updateHttpCookies


http://gerrit.cloudera.org:8080/#/c/17667/3/shell/ImpalaHttpClient.py@184
PS3, Line 184: self.__http_cookie_dict[cn] = Cookie(cookie=None, expiry_time=None)
nit: can we just remove the entry from __http_cookie_dict?


http://gerrit.cloudera.org:8080/#/c/17667/3/shell/ImpalaHttpClient.py@194
PS3, Line 194: addHttpCookie
nit: addHttpCookiesToRequestHeaders



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jul 2021 19:39:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/17667 )

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................

IMPALA-10784: Add support for retaining cookies in impala-shell

IMPALA-10234 added support for cookie authentication for LDAP to
impala-shell. But it does not accept user input cookie name via
startup flags, and it retains only one cookie.

In some scenarios, we could use proxy to manage the sessions with
additional HTTP cookies added by proxy.
This patch made cookie support more generic for impala-shell.
It lets the user specify cookie names via a startup flag
"--http_cookie_names" and could retain more than one cookies.

Testing:
 - Passed impala-shell unit-tests.
 - Manualy tested the multiple cookies in HTTP headers with a
   customized Impala server which could send and receive multiple
   cookies.
 - TODO: new test cases.

Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
---
M shell/ImpalaHttpClient.py
M shell/cookie_util.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
M tests/shell/test_cookie_util.py
6 files changed, 97 insertions(+), 64 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 12 Jul 2021 18:01:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 4:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Sat, 10 Jul 2021 02:40:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17667/1/tests/shell/test_cookie_util.py
File tests/shell/test_cookie_util.py:

http://gerrit.cloudera.org:8080/#/c/17667/1/tests/shell/test_cookie_util.py@93
PS1, Line 93: c
flake8: F841 local variable 'c' is assigned to but never used



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jul 2021 06:40:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jul 2021 07:09:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/17667 )

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................

IMPALA-10784: Add support for retaining cookies in impala-shell

IMPALA-10234 added support for cookie authentication for LDAP to
impala-shell. But it does not accept user input cookie name via
startup flags, and it retains only one cookie.

In some scenarios, we could use proxy to manage the sessions with
additional HTTP cookies added by proxy.
This patch made cookie support more generic for impala-shell.
It lets the user specify cookie names via a startup flag
"--http_cookie_names" and could retain more than one cookies.

Testing:
 - Manualy tested the multiple cookies in HTTP headers with a
   customized Impala server which could send and receive multiple
   cookies.
 - Passed core test, including new test cases.

Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
---
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/cookie_util.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
M tests/shell/test_cookie_util.py
7 files changed, 146 insertions(+), 67 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 12 Jul 2021 18:01:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17667 )

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................

IMPALA-10784: Add support for retaining cookies in impala-shell

IMPALA-10234 added support for cookie authentication for LDAP to
impala-shell. But it does not accept user input cookie name via
startup flags, and it retains only one cookie.

In some scenarios, we could use proxy to manage the sessions with
additional HTTP cookies added by proxy.
This patch made cookie support more generic for impala-shell.
It lets the user specify cookie names via a startup flag
"--http_cookie_names" and could retain more than one cookies.

Testing:
 - Manualy tested the multiple cookies in HTTP headers with a
   customized Impala server which could send and receive multiple
   cookies.
 - Passed core test, including new test cases.

Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Reviewed-on: http://gerrit.cloudera.org:8080/17667
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/cookie_util.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
M tests/shell/test_cookie_util.py
7 files changed, 146 insertions(+), 67 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 5:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 12 Jul 2021 01:17:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jul 2021 07:00:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/17667 )

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................

IMPALA-10784: Add support for retaining cookies in impala-shell

IMPALA-10234 added support for cookie authentication for LDAP to
impala-shell. But it does not accept user input cookie name via
startup flags, and it retains only one cookie.

In some scenarios, we could use proxy to manage the sessions with
additional HTTP cookies added by proxy.
This patch made cookie support more generic for impala-shell.
It lets the user specify cookie names via a startup flag
"--http_cookie_names" and could retain more than one cookies.

Testing:
 - Manualy tested the multiple cookies in HTTP headers with a
   customized Impala server which could send and receive multiple
   cookies.
 - Passed core test, including new test cases.

Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
---
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/cookie_util.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
M tests/shell/test_cookie_util.py
7 files changed, 131 insertions(+), 67 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17667/2/tests/shell/test_cookie_util.py
File tests/shell/test_cookie_util.py:

http://gerrit.cloudera.org:8080/#/c/17667/2/tests/shell/test_cookie_util.py@93
PS2, Line 93: =
flake8: E225 missing whitespace around operator



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jul 2021 06:45:59 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17667/2/tests/shell/test_cookie_util.py
File tests/shell/test_cookie_util.py:

http://gerrit.cloudera.org:8080/#/c/17667/2/tests/shell/test_cookie_util.py@93
PS2, Line 93: =
> flake8: E225 missing whitespace around operator
fixed



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jul 2021 06:49:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/17667 )

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................

IMPALA-10784: Add support for retaining cookies in impala-shell

IMPALA-10234 added support for cookie authentication for LDAP to
impala-shell. But it does not accept user input cookie name via
startup flags, and it retains only one cookie.

In some scenarios, we could use proxy to manage the sessions with
additional HTTP cookies added by proxy.
This patch made cookie support more generic for impala-shell.
It lets the user specify cookie names via a startup flag
"--http_cookie_names" and could retain more than one cookies.

Testing:
 - Passed impala-shell unit-tests.
 - Manualy tested the multiple cookies in HTTP headers with a
   customized Impala server which could send and receive multiple
   cookies.
 - TODO: new test cases.

Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
---
M shell/ImpalaHttpClient.py
M shell/cookie_util.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
M tests/shell/test_cookie_util.py
6 files changed, 97 insertions(+), 64 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/17667/3/shell/ImpalaHttpClient.py
File shell/ImpalaHttpClient.py:

http://gerrit.cloudera.org:8080/#/c/17667/3/shell/ImpalaHttpClient.py@66
PS3, Line 66: http_cookie_names is comma-separated list of cookie names which are used to specify
            :     the cookie-based authentication or session management
> nit: http_cookie_names is used to specify a comma-separated list of possibl
fixed.


http://gerrit.cloudera.org:8080/#/c/17667/3/shell/ImpalaHttpClient.py@170
PS3, Line 170: updateHttpCookie
> nit: updateHttpCookies
fixed


http://gerrit.cloudera.org:8080/#/c/17667/3/shell/ImpalaHttpClient.py@184
PS3, Line 184: self.__http_cookie_dict[cn] = Cookie(cookie=None, expiry_time=None)
> nit: can we just remove the entry from __http_cookie_dict?
We did not save http_cookie_names after create self.__http_cookie_dict. We use self.__http_cookie_dict.keys() when calling get_all_matching_cookies() so we cannot remove the entry.


http://gerrit.cloudera.org:8080/#/c/17667/3/shell/ImpalaHttpClient.py@194
PS3, Line 194: addHttpCookie
> nit: addHttpCookiesToRequestHeaders
fixed



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Sat, 10 Jul 2021 02:13:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Jul 2021 00:07:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10784: Add support for retaining cookies in impala-shell

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

Change subject: IMPALA-10784: Add support for retaining cookies in impala-shell
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I193422d5ec891886a522d82ecb0e9d974132ff2a
Gerrit-Change-Number: 17667
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 12 Jul 2021 16:15:08 +0000
Gerrit-HasComments: No