[impala] 01/02: IMPALA-9909: Print body of http error code in Impala Shell.

[ta...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit e187c4054340ed8f00c35711185a2fd6719f48b3
Author: Andrew Sherman <as...@cloudera.com>
AuthorDate: Wed May 20 11:57:22 2020 -0700

    IMPALA-9909: Print body of http error code in Impala Shell.
    
    Make Impala Shell closer to Impyla by printing the body of any http
    error code message received when using hs2-over-http. The common case is
    that there is nothing in the body, in which case the behavior is
    unchanged.
    
    TESTING
     Added a test for the new functionality.
     Ran all end-to-end tests.
    
    Change-Id: Iabc45eda0b87ca694b8359148cda6a7c1d5a8fff
    Reviewed-on: http://gerrit.cloudera.org:8080/16269
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 shell/ImpalaHttpClient.py             |  10 +++-
 tests/shell/test_shell_interactive.py | 105 ++++++++++++++++++++++++++--------
 2 files changed, 90 insertions(+), 25 deletions(-)

diff --git a/shell/ImpalaHttpClient.py b/shell/ImpalaHttpClient.py
index cd79971..1820a1b 100644
--- a/shell/ImpalaHttpClient.py
+++ b/shell/ImpalaHttpClient.py
@@ -36,6 +36,7 @@ import six
 # The current changes that have been applied:
 # - Added logic for the 'Expect: 100-continue' header on large requests
 # - If an error code is received back in flush(), an exception is thrown.
+# Note there is a copy of this code in Impyla.
 class ImpalaHttpClient(TTransportBase):
   """Http implementation of TTransport base."""
 
@@ -151,6 +152,9 @@ class ImpalaHttpClient(TTransportBase):
   def read(self, sz):
     return self.__http_response.read(sz)
 
+  def readBody(self):
+    return self.__http_response.read()
+
   def write(self, buf):
     self.__wbuf.write(buf)
 
@@ -208,4 +212,8 @@ class ImpalaHttpClient(TTransportBase):
     if self.code >= 300:
       # Report any http response code that is not 1XX (informational response) or
       # 2XX (successful).
-      raise RPCException("HTTP code {}: {}".format(self.code, self.message))
+      body = self.readBody()
+      if not body:
+        raise RPCException("HTTP code {}: {}".format(self.code, self.message))
+      else:
+        raise RPCException("HTTP code {}: {} [{}]".format(self.code, self.message, body))
diff --git a/tests/shell/test_shell_interactive.py b/tests/shell/test_shell_interactive.py
index 245fb9b..f9668d6 100755
--- a/tests/shell/test_shell_interactive.py
+++ b/tests/shell/test_shell_interactive.py
@@ -76,43 +76,85 @@ def tmp_history_file(request):
   return tmp.name
 
 
-@pytest.yield_fixture
-def http_503_server():
-  class RequestHandler503(SimpleHTTPServer.SimpleHTTPRequestHandler):
-    """A custom http handler that checks for duplicate 'Host' headers from the most
-    recent http request, and always returns a 503 http code"""
+class RequestHandler503(SimpleHTTPServer.SimpleHTTPRequestHandler):
+  """A custom http handler that checks for duplicate 'Host' 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
+  def __init__(self, request, client_address, server):
+    SimpleHTTPServer.SimpleHTTPRequestHandler.__init__(self, request, client_address,
+                                                       server)
 
-      # 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
+  def should_send_body_text(self):
+    # in RequestHandler503 we do not send any body text
+    return False
 
-      # Respond with 503.
-      self.send_response(code=httplib.SERVICE_UNAVAILABLE, message="Service Unavailable")
+  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
 
-  class TestHTTPServer503(object):
-    def __init__(self):
-      self.HOST = "localhost"
-      self.PORT = get_unused_port()
-      self.httpd = SocketServer.TCPServer((self.HOST, self.PORT), RequestHandler503)
+    # 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
 
-      self.http_server_thread = threading.Thread(target=self.httpd.serve_forever)
-      self.http_server_thread.start()
+    # Respond with 503.
+    self.send_response(code=httplib.SERVICE_UNAVAILABLE, message="Service Unavailable")
+    if self.should_send_body_text():
+      # Optionally send ody text with 503 message.
+      self.end_headers()
+      self.wfile.write("EXTRA")
 
-  server = TestHTTPServer503()
-  yield server
 
-  # Cleanup after test.
+class RequestHandler503Extra(RequestHandler503):
+  """"Override RequestHandler503 so as to send body text with the 503 message."""
+
+  def __init__(self, request, client_address, server):
+    RequestHandler503.__init__(self, request, client_address, server)
+
+  def should_send_body_text(self):
+    # in RequestHandler503Extra we will send body text
+    return True
+
+
+class TestHTTPServer503(object):
+  def __init__(self, clazz):
+    self.HOST = "localhost"
+    self.PORT = get_unused_port()
+    self.httpd = SocketServer.TCPServer((self.HOST, self.PORT), clazz)
+
+    self.http_server_thread = threading.Thread(target=self.httpd.serve_forever)
+    self.http_server_thread.start()
+
+
+def shutdown_server(server):
+  """Helper method to shutdown a http server."""
   if server.httpd is not None:
     server.httpd.shutdown()
   if server.http_server_thread is not None:
     server.http_server_thread.join()
 
 
+@pytest.yield_fixture
+def http_503_server():
+  """A fixture that creates an http server that returns a 503 http code."""
+  server = TestHTTPServer503(RequestHandler503)
+  yield server
+
+  # Cleanup after test.
+  shutdown_server(server)
+
+
+@pytest.yield_fixture
+def http_503_server_extra():
+  """A fixture that creates an http server that returns a 503 http code with extra
+  body text."""
+  server = TestHTTPServer503(RequestHandler503Extra)
+  yield server
+
+  # Cleanup after test.
+  shutdown_server(server)
+
+
 class TestImpalaShellInteractive(ImpalaTestSuite):
   """Test the impala shell interactively"""
 
@@ -1026,6 +1068,21 @@ class TestImpalaShellInteractive(ImpalaTestSuite):
     shell_proc = spawn_shell([IMPALA_SHELL_EXECUTABLE] + shell_args)
     shell_proc.expect("HTTP code 503", timeout=10)
 
+  def test_http_interactions_extra(self, vector, http_503_server_extra):
+    """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,
+    including the body text from the message."""
+    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_extra.HOST,
+                                     http_503_server_extra.PORT)]
+    shell_proc = spawn_shell([IMPALA_SHELL_EXECUTABLE] + shell_args)
+    shell_proc.expect("HTTP code 503: Service Unavailable \[EXTRA\]", timeout=10)
+
 
 def run_impala_shell_interactive(vector, input_lines, shell_args=None,
                                  wait_until_connected=True):