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

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


The following commit(s) were added to refs/heads/master by this push:
     new e1098a6a0 IMPALA-13214: Skip wait_until_connected when shell exits
e1098a6a0 is described below

commit e1098a6a02c417ddc63904259fa0abbcc64fcdb7
Author: Michael Smith <[email protected]>
AuthorDate: Thu Jul 18 11:40:36 2024 -0700

    IMPALA-13214: Skip wait_until_connected when shell exits
    
    The ImpalaShell class expects to start impala-shell and interact with it
    by sending instructions over stdin and reading the results. This
    assumption was incorrect when used for impala-shell batch sessions,
    where the process exits on its own. If there's a delay in
    ImpalaShell.__init__ - between starting the process and polling to see
    that it's running - for a batch process, ImpalaShell will fail the
    assertion that process_status is None. This can be easily reproduced by
    adding a small (0.1s) sleep after starting the new process.
    
    Most batch runs of impala-shell happen through `run_impala_shell_cmd`.
    Updated that function to only wait for a successful connection when
    stdin input is supplied. Otherwise the command is assumed to be a batch
    function and any failures will be detected during `get_result`. Removed
    explicit use of `wait_until_connected` as redundant.
    
    Fixed cases in test_config_file that previously ignored WARNING before
    the connection string because they did not specify
    `wait_until_connected`.
    
    Tested by running shell/test_shell_commandline.py with a 0.1s delay
    before ImpalaShell polls.
    
    Change-Id: I24e029b6192a17773760cb44fd7a4f87b71c0aae
    Reviewed-on: http://gerrit.cloudera.org:8080/21598
    Tested-by: Impala Public Jenkins <[email protected]>
    Reviewed-by: Jason Fehr <[email protected]>
    Reviewed-by: Kurt Deschler <[email protected]>
---
 tests/custom_cluster/test_client_ssl.py |  7 +++----
 tests/shell/test_shell_commandline.py   | 37 ++++++++++++++++++---------------
 tests/shell/util.py                     | 11 ++++++----
 3 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/tests/custom_cluster/test_client_ssl.py 
b/tests/custom_cluster/test_client_ssl.py
index 1a23a9872..51f3b2afe 100644
--- a/tests/custom_cluster/test_client_ssl.py
+++ b/tests/custom_cluster/test_client_ssl.py
@@ -247,7 +247,7 @@ class TestClientSsl(CustomClusterTestSuite):
   def _validate_positive_cases(self, vector, ca_cert=""):
     python3_10_version_re = re.compile(r"using Python 3\.1[0-9]")
     shell_options = ["--ssl", "-q", "select 1 + 2"]
-    result = run_impala_shell_cmd(vector, shell_options, 
wait_until_connected=False)
+    result = run_impala_shell_cmd(vector, shell_options)
     for msg in [self.SSL_ENABLED, self.CONNECTED, self.FETCHED]:
       assert msg in result.stderr
     # Python >3.10 has deprecated ssl.PROTOCOL_TLS and impala-shell currently 
emits a
@@ -260,7 +260,7 @@ class TestClientSsl(CustomClusterTestSuite):
 
     if ca_cert != "":
       shell_options = shell_options + ["--ca_cert=%s" % ca_cert]
-      result = run_impala_shell_cmd(vector, shell_options, 
wait_until_connected=False)
+      result = run_impala_shell_cmd(vector, shell_options)
       for msg in [self.SSL_ENABLED, self.CONNECTED, self.FETCHED]:
         assert msg in result.stderr
       if not python3_10_version_re.search(result.stderr):
@@ -294,7 +294,6 @@ class TestClientSslUnsupported(CustomClusterTestSuite):
                                     catalogd_args=SSL_ARGS)
   @pytest.mark.skipif(SKIP_SSL_MSG is not None, reason=SKIP_SSL_MSG)
   def test_shell_warning(self, vector):
-    result = run_impala_shell_cmd_no_expect(
-      vector, ["--ssl", "-q", "select 1 + 2"], wait_until_connected=False)
+    result = run_impala_shell_cmd_no_expect(vector, ["--ssl", "-q", "select 1 
+ 2"])
     assert "Warning: TLSv1.2 is not supported for Python < 2.7.9" in 
result.stderr, \
       result.stderr
diff --git a/tests/shell/test_shell_commandline.py 
b/tests/shell/test_shell_commandline.py
index 3f348ba71..983e95e52 100644
--- a/tests/shell/test_shell_commandline.py
+++ b/tests/shell/test_shell_commandline.py
@@ -183,11 +183,11 @@ class TestImpalaShell(ImpalaTestSuite):
   def test_unsecure_message(self, vector):
     if vector.get_value('strict_hs2_protocol'):
       pytest.skip("Strict protocol always runs with LDAP.")
-    results = run_impala_shell_cmd(vector, [], wait_until_connected=False)
+    results = run_impala_shell_cmd(vector, [])
     assert "with no authentication" in results.stderr
 
   def test_fastbinary_warning_message(self, vector):
-    results = run_impala_shell_cmd(vector, [], wait_until_connected=False)
+    results = run_impala_shell_cmd(vector, [])
     # Verify that we don't print any message about fastbinary
     # This doesn't check the full error string, because that could change.
     assert "fastbinary" not in results.stderr
@@ -729,8 +729,7 @@ class TestImpalaShell(ImpalaTestSuite):
 
     # Testing config file related warning and error messages
     args = ['--config_file=%s/impalarc_with_warnings' % QUERY_FILE_PATH]
-    result = run_impala_shell_cmd(
-        vector, args, expect_success=True, wait_until_connected=False)
+    result = run_impala_shell_cmd(vector, args, expect_success=True)
     assert "WARNING: Option 'config_file' can be only set from shell." in 
result.stderr
     err_msg = ("WARNING: Unable to read configuration file correctly. "
                "Ignoring unrecognized config option: 'invalid_option'\n")
@@ -742,21 +741,25 @@ class TestImpalaShell(ImpalaTestSuite):
                "'maybe' is not a valid value for a boolean option.")
     assert  err_msg in result.stderr
 
-    # Test the optional configuration file with live_progress and live_summary
-    # Positive test
-    args = ['--config_file=%s/good_impalarc3' % QUERY_FILE_PATH]
-    result = run_impala_shell_cmd(vector, args)
-    assert 'WARNING:' not in result.stderr, \
-      "A valid config file should not trigger any warning: 
{0}".format(result.stderr)
-    # Negative Tests
-    # specified config file with live_summary enabled for non interactive mode
-    args = ['--config_file=%s/good_impalarc3' % QUERY_FILE_PATH, 
'--query=select 3']
-    result = run_impala_shell_cmd(vector, args, expect_success=False)
-    assert 'live_summary is available for interactive mode only' in 
result.stderr
+    # live_progress and live_summary are not supported with strict_hs2_protocol
+    if not vector.get_value('strict_hs2_protocol'):
+      # Test the optional configuration file with live_progress and 
live_summary
+      # Positive test
+      args = ['--config_file=%s/good_impalarc3' % QUERY_FILE_PATH]
+      result = run_impala_shell_cmd(vector, args)
+      filtered_stderr = [line for line in result.stderr.splitlines()
+                         if 'WARNING: Unable to load command history' not in 
line]
+      assert 'WARNING:' not in '\n'.join(filtered_stderr), \
+        "A valid config file should not trigger any warning: 
{0}".format(result.stderr)
+      # Negative Tests
+      # specified config file with live_summary enabled for non interactive 
mode
+      args = ['--config_file=%s/good_impalarc3' % QUERY_FILE_PATH, 
'--query=select 3']
+      result = run_impala_shell_cmd(vector, args, expect_success=False)
+      assert 'live_summary is available for interactive mode only' in 
result.stderr
+
     # testing config file related warning messages
     args = ['--config_file=%s/impalarc_with_warnings2' % QUERY_FILE_PATH]
-    result = run_impala_shell_cmd(
-      vector, args, expect_success=True, wait_until_connected=False)
+    result = run_impala_shell_cmd(vector, args, expect_success=True)
     err_msg = ("WARNING: Unable to read configuration file correctly. "
                "Ignoring unrecognized config option: 'Live_Progress'\n")
     assert err_msg in result.stderr
diff --git a/tests/shell/util.py b/tests/shell/util.py
index 889c075c8..447c9418b 100755
--- a/tests/shell/util.py
+++ b/tests/shell/util.py
@@ -122,15 +122,14 @@ def assert_pattern(pattern, result, text, message):
 
 
 def run_impala_shell_cmd(vector, shell_args, env=None, expect_success=True,
-                         stdin_input=None, wait_until_connected=True,
-                         stdout_file=None, stderr_file=None):
+                         stdin_input=None, stdout_file=None, stderr_file=None):
   """Runs the Impala shell on the commandline.
 
   'shell_args' is a string which represents the commandline options.
   Returns a ImpalaShellResult.
   """
   result = run_impala_shell_cmd_no_expect(vector, shell_args, env, stdin_input,
-                                          expect_success and 
wait_until_connected,
+                                          wait_until_connected=expect_success,
                                           stdout_file=stdout_file,
                                           stderr_file=stderr_file)
   if expect_success:
@@ -151,7 +150,11 @@ def run_impala_shell_cmd_no_expect(vector, shell_args, 
env=None, stdin_input=Non
 
   Does not assert based on success or failure of command.
   """
-  p = ImpalaShell(vector, shell_args, env=env, 
wait_until_connected=wait_until_connected,
+  # Only wait_until_connected if we also have input to send. Otherwise expect 
that
+  # the command will run without input and exit. Avoid waiting as ImpalaShell 
may
+  # exit before it can detect there is a successful connection and assert 
False.
+  p = ImpalaShell(vector, shell_args, env=env,
+                  wait_until_connected=wait_until_connected and stdin_input,
                   stdout_file=stdout_file, stderr_file=stderr_file)
   result = p.get_result(stdin_input)
   return result

Reply via email to