You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/03/22 16:14:07 UTC

[2/2] impala git commit: IMPALA-6716: Store LDAP options as shell member variables

IMPALA-6716: Store LDAP options as shell member variables

When passing comamnd line options to a new instance of the
ImpalaShell, we ususally transfer the options to member
variables of that new instance. We weren't doing that with
all of the LDAP-related options, even though we wanted to
access them later. In some environments and under certain
conditions, this could then lead to a NameError exception
being thrown.

This patch takes away any reliance on the original options
object returned by parse_args() beyond the __init__()
method of the ImpalaShell class, by tranferring all LDAP
options to member variables. Also, a test has been added to
exercise the code path where the exception had been occurring.

Change-Id: I810850f569ef3f4487f7eeba81ca520dc955ac2e
Reviewed-on: http://gerrit.cloudera.org:8080/9744
Reviewed-by: Michael Brown <mi...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/08b60a15
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/08b60a15
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/08b60a15

Branch: refs/heads/master
Commit: 08b60a15cc9548487e1d62ecefde7672a20a8487
Parents: 942de72
Author: David Knupp <dk...@cloudera.com>
Authored: Tue Mar 20 23:00:14 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Mar 22 10:58:40 2018 +0000

----------------------------------------------------------------------
 shell/impala_shell.py                 |  7 ++++---
 tests/shell/test_shell_interactive.py | 13 +++++++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/08b60a15/shell/impala_shell.py
----------------------------------------------------------------------
diff --git a/shell/impala_shell.py b/shell/impala_shell.py
index 43620a8..f13899d 100755
--- a/shell/impala_shell.py
+++ b/shell/impala_shell.py
@@ -146,6 +146,7 @@ class ImpalaShell(object, cmd.Cmd):
     self.ca_cert = options.ca_cert
     self.user = options.user
     self.ldap_password = options.ldap_password
+    self.ldap_password_cmd = options.ldap_password_cmd
     self.use_ldap = options.use_ldap
 
     self.verbose = options.verbose
@@ -819,9 +820,9 @@ class ImpalaShell(object, cmd.Cmd):
         print_to_stderr("Socket error %s: %s" % (code, e))
         self.prompt = self.DISCONNECTED_PROMPT
     except Exception, e:
-      if options.ldap_password_cmd and \
-          options.ldap_password and \
-          options.ldap_password.endswith('\n'):
+      if self.ldap_password_cmd and \
+          self.ldap_password and \
+          self.ldap_password.endswith('\n'):
         print_to_stderr("Warning: LDAP password contains a trailing newline. "
                       "Did you use 'echo' instead of 'echo -n'?")
       print_to_stderr("Error connecting: %s, %s" % (type(e).__name__, e))

http://git-wip-us.apache.org/repos/asf/impala/blob/08b60a15/tests/shell/test_shell_interactive.py
----------------------------------------------------------------------
diff --git a/tests/shell/test_shell_interactive.py b/tests/shell/test_shell_interactive.py
index 087e40f..17eac5b 100755
--- a/tests/shell/test_shell_interactive.py
+++ b/tests/shell/test_shell_interactive.py
@@ -28,6 +28,12 @@ import sys
 import tempfile
 from time import sleep
 
+# This import is the actual ImpalaShell class from impala_shell.py.
+# We rename it to ImpalaShellClass here because we later import another
+# class called ImpalaShell from tests/shell/util.py, and we don't want
+# to mask it.
+from shell.impala_shell import ImpalaShell as ImpalaShellClass
+
 from tests.common.impala_service import ImpaladService
 from tests.common.skip import SkipIfLocal
 from util import assert_var_substitution, ImpalaShell
@@ -131,6 +137,13 @@ class TestImpalaShellInteractive(object):
     assert result.stdout.count("Welcome to the Impala shell") == 1
 
   @pytest.mark.execute_serially
+  def test_disconnected_shell(self):
+    """Test that the shell presents a disconnected prompt if it can't connect
+    """
+    result = run_impala_shell_interactive('asdf;', shell_args='-i foo')
+    assert ImpalaShellClass.DISCONNECTED_PROMPT in result.stdout
+
+  @pytest.mark.execute_serially
   def test_bash_cmd_timing(self):
     """Test existence of time output in bash commands run from shell"""
     args = "! ls;"