You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Andrew Sherman (Code Review)" <ge...@cloudera.org> on 2020/04/18 00:16:05 UTC

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

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 <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>

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

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Apr 2020 18:19:54 +0000
Gerrit-HasComments: Yes

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

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2020 20:28:01 +0000
Gerrit-HasComments: Yes

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

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Apr 2020 02:51:45 +0000
Gerrit-HasComments: Yes

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 29 Jun 2020 21:36:39 +0000
Gerrit-HasComments: No

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 29 Jun 2020 21:03:43 +0000
Gerrit-HasComments: Yes

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 29 Jun 2020 21:09:59 +0000
Gerrit-HasComments: Yes

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

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jun 2020 16:42:08 +0000
Gerrit-HasComments: No

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jun 2020 16:45:13 +0000
Gerrit-HasComments: No

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

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Apr 2020 02:52:32 +0000
Gerrit-HasComments: Yes

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jun 2020 21:46:25 +0000
Gerrit-HasComments: No

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

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 May 2020 23:47:32 +0000
Gerrit-HasComments: No

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

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

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

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/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 <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

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

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

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

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jun 2020 16:45:14 +0000
Gerrit-HasComments: No

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

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 29 Jun 2020 21:43:43 +0000
Gerrit-HasComments: No

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 18 Apr 2020 00:58:24 +0000
Gerrit-HasComments: No

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 May 2020 00:15:49 +0000
Gerrit-HasComments: No

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

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 May 2020 23:38:01 +0000
Gerrit-HasComments: Yes

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 May 2020 23:35:47 +0000
Gerrit-HasComments: Yes

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

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 29 Jun 2020 21:30:26 +0000
Gerrit-HasComments: No