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