[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-06-30 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 30 Jun 2020 21:46:25 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-06-30 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..

IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in 
http mode.

Many http servers will not accept an http request that has multiple
copies of the "Host" header. A recent toolchain change patches
Thrift so that will not send the extraneous header (in THttpClient).
This change tests that the duplicate headers are not sent,

TESTING:
  Ran all end-to-end tests.
  Rewrote an existing Shell test to check that only one "Host" header
  is sent.

Change-Id: I82996015d0205923e854dac8bb88604778684c46
Reviewed-on: http://gerrit.cloudera.org:8080/15752
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
2 files changed, 54 insertions(+), 35 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-06-30 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 30 Jun 2020 16:45:13 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-06-30 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 30 Jun 2020 16:45:14 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-06-30 Thread Andrew Sherman (Code Review)
Andrew Sherman has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 5: Code-Review+2

Bring forward +2
I rebased and ran all tests


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 30 Jun 2020 16:42:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-06-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 5:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 29 Jun 2020 21:43:43 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-06-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 4:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 29 Jun 2020 21:36:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-06-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 3:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 29 Jun 2020 21:30:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-06-29 Thread Andrew Sherman (Code Review)
Andrew Sherman has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..

IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in 
http mode.

Many http servers will not accept an http request that has multiple
copies of the "Host" header. A recent toolchain change patches
Thrift so that will not send the extraneous header (in THttpClient).
This change tests that the duplicate headers are not sent,

TESTING:
  Ran all end-to-end tests.
  Rewrote an existing Shell test to check that only one "Host" header
  is sent.

Change-Id: I82996015d0205923e854dac8bb88604778684c46
---
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
2 files changed, 54 insertions(+), 35 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-06-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15752/4/tests/shell/util.py
File tests/shell/util.py:

http://gerrit.cloudera.org:8080/#/c/15752/4/tests/shell/util.py@325
PS4, Line 325:
flake8: W391 blank line at end of file



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 29 Jun 2020 21:09:59 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-06-29 Thread Andrew Sherman (Code Review)
Andrew Sherman has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..

IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in 
http mode.

Many http servers will not accept an http request that has multiple
copies of the "Host" header. A recent toolchain change patches
Thrift so that will not send the extraneous header (in THttpClient).
This change tests that the duplicate headers are not sent,

TESTING:
  Ran all end-to-end tests.
  Rewrote an existing Shell test to check that only one "Host" header
  is sent.

Change-Id: I82996015d0205923e854dac8bb88604778684c46
---
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
2 files changed, 55 insertions(+), 35 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-06-29 Thread Andrew Sherman (Code Review)
Andrew Sherman has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..

IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in 
http mode.

Many http servers will not accept an http request that has multiple
copies of the "Host" header. A recent toolchain change patches
Thrift so that will not send the extraneous header (in THttpClient).
This change tests that the duplicate headers are not sent,

TESTING:
  Ran all end-to-end tests.
  Rewrote an existing Shell test to check that only one "Host" header
  is sent.

Change-Id: I82996015d0205923e854dac8bb88604778684c46
---
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
2 files changed, 54 insertions(+), 35 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-06-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/15752/3/tests/shell/util.py@324
PS3, Line 324:
flake8: W292 no newline at end of file



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 29 Jun 2020 21:03:43 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-05-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 2:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 06 May 2020 00:15:49 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-05-05 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 2: Code-Review+2

Well, since you're just copying my code, of course I'll +2 it. ;-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 05 May 2020 23:47:32 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-05-05 Thread Andrew Sherman (Code Review)
Andrew Sherman has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 1:

(1 comment)

I have pretty much copied what you suggested :-)

http://gerrit.cloudera.org:8080/#/c/15752/1/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/15752/1/tests/shell/test_shell_interactive.py@88
PS1, Line 88: self.headers.headers
> Sorry about the line wrapping.
Thanks for the really useful review.
The thing I had not understood was how useful it is to use a
@pytest.yield_fixture
even for a one-use situation.
And they don't have to be in conftest.py



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 05 May 2020 23:38:01 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-05-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15752/2/tests/shell/util.py
File tests/shell/util.py:

http://gerrit.cloudera.org:8080/#/c/15752/2/tests/shell/util.py@251
PS2, Line 251:
flake8: W292 no newline at end of file



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 05 May 2020 23:35:47 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-05-05 Thread Andrew Sherman (Code Review)
Andrew Sherman has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..

IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in 
http mode.

Many http servers will not accept an http request that has multiple
copies of the "Host" header. A recent toolchain change patches
Thrift so that will not send the extraneous header (in THttpClient).
This change tests that the duplicate headers are not sent,

TESTING:
  Ran all end-to-end tests.
  Rewrote an existing Shell test to check that only one "Host" header
  is sent.

Change-Id: I82996015d0205923e854dac8bb88604778684c46
---
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
2 files changed, 53 insertions(+), 34 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-04-29 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15752/1/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/15752/1/tests/shell/test_shell_interactive.py@88
PS1, Line 88: self.headers.headers
> Sorry, it took me a while to reason through this. I have a couple of though
Sorry about the line wrapping.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 30 Apr 2020 02:52:32 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-04-29 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15752/1/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/15752/1/tests/shell/test_shell_interactive.py@88
PS1, Line 88: self.headers.headers
> OK headers.headers does look weird.
Sorry, it took me a while to reason through this. I have a couple of thoughts. 
This first is that both the TestRequestHandler and the test could probably be 
simplified if you moved the assert about the number of "Host:" headers into the 
handler itself. I'm not sure that storing the headers temporarily only to check 
them from some external context buys you much, but it makes the code harder to 
follow. So maybe something like this:

  # yeah, I changed the name
  class RequestHandler503(SimpleHTTPServer.SimpleHTTPRequestHandler):
"""A custom http handler that stores the headers from the most recent http 
request,
and always returns a 503 http code"""

def do_POST(self):
  # The unfortunately named self.headers here is an instance of 
mimetools.Message that
  # contains the request headers
  request_headers = self.headers.headers

  # Ensure that only one 'Host' header is contained in the request before 
responding
  host_hdr_count = sum([header.startswith('Host:') for header in 
request_headers])
  assert host_hdr_count == 1, "duplicate 'Host:' headers in %s" % 
request_headers

  # respond with 503.
  self.send_response(code=httplib.SERVICE_UNAVAILABLE, message="Service 
Unavailable")


The other thing that I thought about (and this is on me, as someone who 
reviewed the earlier incarnation of this test) is that you really could also 
clean up the test quite a bit if you used a fixture for the server. In general, 
if you find that you have a lot of boilerplate setup/teardown code in the body 
of a test that has nothing to do with the product being tested, it's a test 
code smell. (Neat aside that supports this -- if there's a failure in the setup 
or teardown within the body of the test, it gets flagged in junit_xml by pytest 
as a FAIL'ed test, i.e., product bug. If the setup/teardown fails from a 
fixture context, it's flagged as an ERROR, which usually indicates framework or 
environment issue. We tend to violate this distinction all over the place.)

Anyway, another approach to further refactor could go so far as to write a 
fixture like this, using the class above.

  @pytest.yield_fixture
  def http_503_server():

class TestHTTPServer503(object):
  def __init__(self):
self.HOST = "localhost"
self.PORT = get_unused_port()
self.httpd = SocketServer.TCPServer((self.HOST, self.PORT), 
RequestHandler503)

self.http_server_thread = 
threading.Thread(target=self.httpd.serve_forever)
self.http_server_thread.start()

server = TestHTTPServer503()
yield server

# cleanup after test
if server.httpd is not None:
  server.httpd.shutdown()
if server.http_server_thread is not None:
  server.http_server_thread.join()


Which would reduce the test to this...

  def test_http_interactions(self, vector, http_503_server):
"""Test interactions with the http server when using hs2-http protocol.
Check that the shell prints a good message when the server returns a 503 
error."""
protocol = vector.get_value("protocol")
if protocol != 'hs2-http':
  pytest.skip()

# Check that we get a message about the 503 error when we try to connect.
shell_args = ["--protocol={0}".format(protocol),
  "-i{0}:{1}".format(http_503_server.HOST, 
http_503_server.PORT)]
shell_proc = pexpect.spawn(IMPALA_SHELL_EXECUTABLE, shell_args)
shell_proc.expect("HTTP code 503", timeout=10)

That's a lot to throw at you. I'd frankly be happy if we could just clean up 
the handler class, if you agree with my recommendation.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 30 Apr 2020 02:51:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-04-22 Thread Andrew Sherman (Code Review)
Andrew Sherman has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 1:

(1 comment)

Let me know if you have more questions about this slightly tricky code.

http://gerrit.cloudera.org:8080/#/c/15752/1/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/15752/1/tests/shell/test_shell_interactive.py@88
PS1, Line 88: self.headers.headers
> Sorry for this potentially obstuse question, but I have to confess, I'm a l
OK headers.headers does look weird.
Here self.headers is an attribute of the SimpleHTTPRequestHandler (actually the 
base class BaseHTTPServer.BaseHTTPRequestHandler). It's type is MeeasageClass 
which inherits from rfc822.Message. This type also has a headers attribute 
which is a simple list of strings.
The TestRequestHandler.headers is a class attribute which I am using as a rock 
to put the saved headers under. This is hacky and would not work in real code 
as that class attribute would be shared by all threads.
I am doing all this so I can save the headers that are received on the server 
side of the http call.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 22 Apr 2020 18:19:54 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-04-20 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15752/1/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/15752/1/tests/shell/test_shell_interactive.py@88
PS1, Line 88: self.headers.headers
Sorry for this potentially obstuse question, but I have to confess, I'm a 
little perplexed by this. "headers" is an attribute that itself has an 
attribute called "headers"? It looks like headers is just a run-of-the-mill 
list. Is the "self" object in this case an instance of the parent class.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 20 Apr 2020 20:28:01 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-04-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15752 )

Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..


Patch Set 1:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Sat, 18 Apr 2020 00:58:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in http mode.

2020-04-17 Thread Andrew Sherman (Code Review)
Andrew Sherman has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/15752


Change subject: IMPALA-9540 Test that Impala Shell no longer sends duplicate 
"Host" headers in http mode.
..

IMPALA-9540 Test that Impala Shell no longer sends duplicate "Host" headers in 
http mode.

Many http servers will not accept an http request that has multiple
copies of the "Host" header. A recent toolchain change patches
Thrift so that will not send the extraneous header (in THttpClient).
This change tests that the duplicate headers are not sent,

TESTING:
  Ran all end-to-end tests.
  Enhanced an existing Shell test to check that only one "Host" header
  is sent.

Change-Id: I82996015d0205923e854dac8bb88604778684c46
---
M tests/shell/test_shell_interactive.py
1 file changed, 23 insertions(+), 7 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I82996015d0205923e854dac8bb88604778684c46
Gerrit-Change-Number: 15752
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: David Knupp