IMPALA-11980 (part 2): Fix absolute import issues for impala_shell

Python 3 changed the behavior of imports with PEP328. Existing
imports become absolute unless they use the new relative import
syntax. This adapts the impala-shell code to use absolute
imports, fixing issues where it is imported from our test code.

There are several parts to this:
1. It moves impala shell code into shell/impala_shell.
   This matches the directory structure of the PyPi package.
2. It changes the imports in the shell code to be
   absolute paths (i.e. impala_shell.foo rather than foo).
   This fixes issues with Python 3 absolute imports.
   It also eliminates the need for ugly hacks in the PyPi
   package's __init__.py.
3. This changes Thrift generation to put it directly in
   $IMPALA_HOME/shell rather than $IMPALA_HOME/shell/gen-py.
   This means that the generated Thrift code is rooted in
   the same directory as the shell code.
4. This changes the PYTHONPATH to include $IMPALA_HOME/shell
   and not $IMPALA_HOME/shell/gen-py. This means that the
   test code is using the same import paths as the pypi
   package.

With all of these changes, the source code is very close
to the directory structure of the PyPi package. As long as
CMake has generated the thrift files and the Python version
file, only a few differences remain. This removes those
differences by moving the setup.py / MANIFEST.in and other
files from the packaging directory to the top-level
shell/ directory. This means that one can pip install
directly from the source code. i.e. pip install $IMPALA_HOME/shell

This also moves the shell tarball generation script to the
packaging directory and changes bin/impala-shell.sh to use
Python 3.

This sorts the imports using isort for the affected Python files.

Testing:
 - Ran a regular core job with Python 2
 - Ran a core job with Python 3 and verified that the absolute
   import issues are gone.

Change-Id: Ica75a24fa6bcb78999b9b6f4f4356951b81c3124
Reviewed-on: http://gerrit.cloudera.org:8080/22330
Reviewed-by: Riza Suminto <riza.suminto@cloudera.com>
Reviewed-by: Michael Smith <michael.smith@cloudera.com>
Tested-by: Riza Suminto <riza.suminto@cloudera.com>

bin/impala-shell.sh

@@ -25,27 +25,16 @@ SHELL_HOME=${IMPALA_SHELL_HOME:-${IMPALA_HOME}/shell}
 
 # ${IMPALA_HOME}/bin has bootstrap_toolchain.py, required by bootstrap_virtualenv.py
 PYTHONPATH=${PYTHONPATH}:${IMPALA_HOME}/bin
-
-# Default version of thrift for the impala-shell is thrift >= 0.11.0.
-PYTHONPATH=${PYTHONPATH}:${SHELL_HOME}/gen-py
-
-THRIFT_PY_ROOT="${IMPALA_TOOLCHAIN_PACKAGES_HOME}/thrift-${IMPALA_THRIFT_PY_VERSION}"
+PYTHONPATH=${PYTHONPATH}:${SHELL_HOME}
 
 export LD_LIBRARY_PATH=":$(PYTHONPATH=${PYTHONPATH} \
   python "$IMPALA_HOME/infra/python/bootstrap_virtualenv.py" \
   --print-ld-library-path)"
 
 IMPALA_PY_DIR="$(dirname "$0")/../infra/python"
-IMPALA_PY_ENV_DIR="${IMPALA_PY_DIR}/env-gcc${IMPALA_GCC_VERSION}"
+IMPALA_PY3_ENV_DIR="${IMPALA_PY_DIR}/env-gcc${IMPALA_GCC_VERSION}-py3"
 # Allow overriding the python executable
-IMPALA_PYTHON_EXECUTABLE="${IMPALA_PYTHON_EXECUTABLE:-${IMPALA_PY_ENV_DIR}/bin/python}"
-
-for PYTHON_LIB_DIR in ${THRIFT_PY_ROOT}/python/lib{64,}; do
-  [[ -d ${PYTHON_LIB_DIR} ]] || continue
-  for PKG_DIR in ${PYTHON_LIB_DIR}/python*/site-packages; do
-    PYTHONPATH=${PYTHONPATH}:${PKG_DIR}/
-  done
-done
+IMPALA_PYTHON_EXECUTABLE="${IMPALA_PYTHON_EXECUTABLE:-${IMPALA_PY3_ENV_DIR}/bin/python3}"
 
 # Note that this uses the external system python executable
 PYTHONPATH=${PYTHONPATH} python "${IMPALA_PY_DIR}/bootstrap_virtualenv.py"
@@ -61,4 +50,4 @@ fi
 
 # This uses the python executable in the impala python env
 PYTHONIOENCODING='utf-8' PYTHONPATH=${PYTHONPATH} \
-  exec "${IMPALA_PYTHON_EXECUTABLE}" ${EXTRA_ARGS:-} ${SHELL_HOME}/impala_shell.py "$@"
+  exec "${IMPALA_PYTHON_EXECUTABLE}" ${EXTRA_ARGS:-} -m "impala_shell.impala_shell" "$@"

bin/rat_exclude_files.txt

@@ -17,15 +17,15 @@ bin/junitxml_prune_notrun.py
 tests/*/__init__.py
 testdata/common/__init__.py
 fe/src/test/resources/regionservers
-shell/__init__.py
+shell/impala_shell/__init__.py
 ssh_keys/id_rsa_impala
 testdata/__init__.py
 tests/__init__.py
 bin/diagnostics/__init__.py
 lib/python/impala_py_lib/__init__.py
 lib/python/impala_py_lib/jenkins/__init__.py
-shell/packaging/MANIFEST.in
-shell/packaging/requirements.txt
+shell/MANIFEST.in
+shell/requirements.txt
 testdata/cluster/node_templates/cdh7/etc/init.d/kms
 testdata/authentication/*
 bin/banned_py3k_warnings.txt
@@ -113,7 +113,7 @@ docker/README.md
 be/src/thirdparty/pcg-cpp-0.98/README.md
 lib/python/README.md
 lib/python/impala_py_lib/gdb/README.md
-shell/packaging/README.md
+shell/README.md
 bin/kerberos/README-kerberos.md
 
 # http://www.apache.org/legal/src-headers.html: "Test data for which the addition of a

bin/set-pythonpath.sh

@@ -25,6 +25,6 @@
 export PYTHONPATH=${IMPALA_HOME}:${IMPALA_HOME}/bin
 
 # Generated Thrift files are used by tests and other scripts.
-PYTHONPATH=${PYTHONPATH}:${IMPALA_HOME}/shell/gen-py
+PYTHONPATH=${PYTHONPATH}:${IMPALA_HOME}/shell
 
 PYTHONPATH=${PYTHONPATH}:${IMPALA_HOME}/infra/python/env-gcc${IMPALA_GCC_VERSION}/lib

common/thrift/CMakeLists.txt

@@ -66,6 +66,9 @@ function(THRIFT_GEN VAR)
         OUTPUT ${OUTPUT_BE_FILE}
         COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_CPP_COMPILER} ${CPP_ARGS} ${THRIFT_FILE}
         COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_PY_COMPILER} ${PYTHON_ARGS} ${THRIFT_FILE}
+        # Ugly hack: Thrift incorrectly generates an unnecessary __init__.py at the top
+        # level. Remove it until we can patch Thrift to avoid generating this.
+        COMMAND rm -f ${PYTHON_OUTPUT_DIR}/__init__.py
         DEPENDS ${ABS_THRIFT_FILE}
         COMMENT "Running thrift compiler on ${THRIFT_FILE}"
         VERBATIM
@@ -83,6 +86,9 @@ function(THRIFT_GEN VAR)
             ${BE_OUTPUT_DIR}/gen-cpp/ImpalaService.h
         COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_JAVA_COMPILER} ${JAVA_FE_ARGS} ${THRIFT_FILE}
         COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_PY_COMPILER} ${PYTHON_ARGS} ${THRIFT_FILE}
+        # Ugly hack: Thrift incorrectly generates an unnecessary __init__.py at the top
+        # level. Remove it until we can patch Thrift to avoid generating this.
+        COMMAND rm -f ${PYTHON_OUTPUT_DIR}/__init__.py
         DEPENDS ${ABS_THRIFT_FILE}
         COMMENT "Running thrift compiler on ${THRIFT_FILE}"
         VERBATIM
@@ -93,6 +99,9 @@ function(THRIFT_GEN VAR)
         COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_CPP_COMPILER} ${CPP_ARGS} ${THRIFT_FILE}
         COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_JAVA_COMPILER} ${JAVA_FE_ARGS} ${THRIFT_FILE}
         COMMAND ${THRIFT_QUIET_WRAPPER} ${THRIFT_PY_COMPILER} ${PYTHON_ARGS} ${THRIFT_FILE}
+        # Ugly hack: Thrift incorrectly generates an unnecessary __init__.py at the top
+        # level. Remove it until we can patch Thrift to avoid generating this.
+        COMMAND rm -f ${PYTHON_OUTPUT_DIR}/__init__.py
         DEPENDS ${ABS_THRIFT_FILE}
         COMMENT "Running thrift compiler on ${THRIFT_FILE}"
         VERBATIM
@@ -164,7 +173,7 @@ file(MAKE_DIRECTORY ${THIRDPARTY_THRIFT_DIR})
 # Args passed to thrift for Java gen
 set(JAVA_FE_ARGS ${THRIFT_JAVA_INCLUDE_DIR_OPTION} --gen java -o ${FE_OUTPUT_DIR})
 set(JAVA_EXT_DS_ARGS ${THRIFT_JAVA_INCLUDE_DIR_OPTION} --gen java -o ${EXT_DS_OUTPUT_DIR})
-set(PYTHON_ARGS ${THRIFT_PY_INCLUDE_DIR_OPTION} -r --gen py:no_utf8strings -o
+set(PYTHON_ARGS ${THRIFT_PY_INCLUDE_DIR_OPTION} -r --gen py:no_utf8strings -out
     ${PYTHON_OUTPUT_DIR})
 
 set (EXT_DATA_SRC_FILES

shell/.gitignore

@@ -1,5 +1,6 @@
 /build/
 /gen-py/
+/impala_thrift_gen/
 /ext-py/
 
 # Ignore the build version python file

shell/CMakeLists.txt

@@ -105,5 +105,5 @@ add_custom_target(shell_python3_install DEPENDS python3_venv shell_pypi_test_pac
 )
 
 add_custom_target(shell_tarball DEPENDS gen-deps shell_pypi_test_package "${IMPALA_PYTHON_BUILD_VENVS}"
-  COMMAND "${CMAKE_SOURCE_DIR}/shell/make_shell_tarball.sh" "${SHELL_TEST_PKG}" ${PYTHON_EXES}
+  COMMAND "${CMAKE_SOURCE_DIR}/shell/packaging/make_shell_tarball.sh" "${SHELL_TEST_PKG}" ${PYTHON_EXES}
 )

shell/MANIFEST.in

@@ -1,3 +1,5 @@
 include *.txt *.md *.py
 recursive-include impala_shell *.py
 recursive-exclude impala_shell *.pyc
+recursive-include impala_thrift_gen *.py
+recursive-exclude impala_thrift_gen *.pyc

shell/gen_impala_build_version.sh

@@ -41,10 +41,10 @@ BUILD_DATE=$(grep "BUILD_TIME: " ${IMPALA_VERSION_INFO_FILE} | cut -f 2- -d ' ')
 cat ${IMPALA_VERSION_INFO_FILE}
 
 SHELL_HOME=${IMPALA_HOME}/shell
-THRIFT_GEN_PY_DIR="${SHELL_HOME}/gen-py"
+IMPALA_SHELL_DIR="${SHELL_HOME}/impala_shell"
 
-rm -f ${THRIFT_GEN_PY_DIR}/impala_build_version.py
-cat > ${THRIFT_GEN_PY_DIR}/impala_build_version.py <<EOF
+rm -f ${IMPALA_SHELL_DIR}/impala_build_version.py
+cat > ${IMPALA_SHELL_DIR}/impala_build_version.py <<EOF
 # -*- coding: utf-8 -*-
 #
 # Licensed to the Apache Software Foundation (ASF) under one

shell/impala_shell/ImpalaHttpClient.py

@@ -16,29 +16,32 @@
 # specific language governing permissions and limitations
 # under the License.
 #
-from __future__ import print_function, unicode_literals
-
+from __future__ import absolute_import, print_function, unicode_literals
+import base64
+from collections import namedtuple
+import datetime
 from io import BytesIO
 import os
 import os.path
 import ssl
 import sys
 import warnings
-import base64
-import datetime
-from collections import namedtuple
-
-from six.moves import urllib, http_client
-
-from thrift.transport.TTransport import TTransportBase
-from shell_exceptions import HttpError, AuthenticationException
-from cookie_util import get_all_matching_cookies, get_all_cookies, get_cookie_expiry
 
 import six
+from six.moves import http_client, urllib
+from thrift.transport.TTransport import TTransportBase
+
+from impala_shell.cookie_util import (
+    get_all_cookies,
+    get_all_matching_cookies,
+    get_cookie_expiry,
+)
+from impala_shell.shell_exceptions import AuthenticationException, HttpError
 
 # Declare namedtuple for Cookie with named fields - cookie and expiry_time
 Cookie = namedtuple('Cookie', ['cookie', 'expiry_time'])
 
+
 # This was taken from THttpClient.py in Thrift to allow making changes Impala needs.
 # The current changes that have been applied:
 # - Added logic for the 'Expect: 100-continue' header on large requests

shell/impala_shell/TSSLSocketWithWildcardSAN.py

@@ -16,18 +16,19 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-from __future__ import print_function, unicode_literals
-
+from __future__ import absolute_import, print_function, unicode_literals
 import re
 import ssl
 
 from thrift.transport import TSSLSocket
 from thrift.transport.TTransport import TTransportException
 
+
 class CertificateError(ValueError):
   """Convenience class to raise errors"""
   pass
 
+
 class TSSLSocketWithWildcardSAN(TSSLSocket.TSSLSocket):
   """
   This is a subclass of thrift's TSSLSocket which has been extended to add the missing

shell/impala_shell/compatibility.py

@@ -17,7 +17,7 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-from __future__ import print_function, unicode_literals
+from __future__ import absolute_import, print_function, unicode_literals
 
 """
 A module where we can aggregate python2 -> 3 code contortions.
@@ -26,7 +26,6 @@ A module where we can aggregate python2 -> 3 code contortions.
 import os
 import sys
 
-
 if sys.version_info.major == 2:
   # default is typically ASCII, but unicode_literals dictates UTF-8
   # See also https://stackoverflow.com/questions/492483/setting-the-correct-encoding-when-piping-stdout-in-python  # noqa

shell/impala_shell/cookie_util.py

@@ -17,6 +17,7 @@
 # under the License.
 #
 
+from __future__ import absolute_import, print_function, unicode_literals
 import datetime
 import os.path
 import sys

shell/impala_shell/impala_client.py

@@ -17,48 +17,67 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-from __future__ import print_function, unicode_literals
-from compatibility import _xrange as xrange
-
-from bitarray import bitarray
+from __future__ import absolute_import, print_function, unicode_literals
 import base64
+from datetime import datetime
 import operator
 import re
-import sasl
 import socket
 import ssl
 import sys
 import time
 import traceback
-from datetime import datetime
 import uuid
 
-from impala_thrift_gen.beeswax import BeeswaxService
-from impala_thrift_gen.beeswax.BeeswaxService import QueryState
-from impala_thrift_gen.ImpalaService import ImpalaService, ImpalaHiveServer2Service
-from impala_thrift_gen.ImpalaService.ImpalaHiveServer2Service import (
-    TGetRuntimeProfileReq, TGetExecSummaryReq, TPingImpalaHS2ServiceReq,
-    TCloseImpalaOperationReq)
-from impala_thrift_gen.ErrorCodes.ttypes import TErrorCode
-from impala_thrift_gen.Status.ttypes import TStatus
-from impala_thrift_gen.TCLIService.TCLIService import (TExecuteStatementReq,
-    TOpenSessionReq, TCloseSessionReq, TProtocolVersion, TStatusCode,
-    TGetOperationStatusReq, TOperationState, TFetchResultsReq, TFetchOrientation,
-    TGetLogReq, TGetResultSetMetadataReq, TTypeId, TCancelOperationReq,
-    TCloseOperationReq)
-from ImpalaHttpClient import ImpalaHttpClient
-from exec_summary import build_exec_summary_table
-from kerberos_util import get_kerb_host_from_kerberos_host_fqdn
+from bitarray import bitarray
+import sasl
 from thrift.protocol import TBinaryProtocol
-from thrift_sasl import TSaslClientTransport
+from thrift.Thrift import TApplicationException, TException
 from thrift.transport.TSocket import TSocket
 from thrift.transport.TTransport import TBufferedTransport, TTransportException
-from thrift.Thrift import TApplicationException, TException
-from shell_exceptions import (RPCException, QueryStateException, DisconnectedException,
-    QueryCancelledByShellException, MissingThriftMethodException, HttpError)
+from thrift_sasl import TSaslClientTransport
 
-from value_converter import HS2ValueConverter
-from thrift_printer import ThriftPrettyPrinter
+from impala_shell.compatibility import _xrange as xrange
+from impala_shell.exec_summary import build_exec_summary_table
+from impala_shell.ImpalaHttpClient import ImpalaHttpClient
+from impala_shell.kerberos_util import get_kerb_host_from_kerberos_host_fqdn
+from impala_shell.shell_exceptions import (
+    DisconnectedException,
+    HttpError,
+    MissingThriftMethodException,
+    QueryCancelledByShellException,
+    QueryStateException,
+    RPCException,
+)
+from impala_shell.thrift_printer import ThriftPrettyPrinter
+from impala_shell.value_converter import HS2ValueConverter
+from impala_thrift_gen.beeswax import BeeswaxService
+from impala_thrift_gen.beeswax.BeeswaxService import QueryState
+from impala_thrift_gen.ErrorCodes.ttypes import TErrorCode
+from impala_thrift_gen.ImpalaService import ImpalaHiveServer2Service, ImpalaService
+from impala_thrift_gen.ImpalaService.ImpalaHiveServer2Service import (
+    TCloseImpalaOperationReq,
+    TGetExecSummaryReq,
+    TGetRuntimeProfileReq,
+    TPingImpalaHS2ServiceReq,
+)
+from impala_thrift_gen.Status.ttypes import TStatus
+from impala_thrift_gen.TCLIService.TCLIService import (
+    TCancelOperationReq,
+    TCloseOperationReq,
+    TCloseSessionReq,
+    TExecuteStatementReq,
+    TFetchOrientation,
+    TFetchResultsReq,
+    TGetLogReq,
+    TGetOperationStatusReq,
+    TGetResultSetMetadataReq,
+    TOpenSessionReq,
+    TOperationState,
+    TProtocolVersion,
+    TStatusCode,
+    TTypeId,
+)
 
 # Getters to extract HS2's representation of values to the display version.
 # An entry must be added to this map for each supported type. HS2's TColumn has many
@@ -96,7 +115,7 @@ def utf8_decode_if_needed(val):
 
 # Helper to decode unicode to utf8 encoded str in Python 2. NOOP in Python 3.
 def utf8_encode_if_needed(val):
-  if sys.version_info.major < 3 and isinstance(val, unicode):
+  if sys.version_info.major < 3 and isinstance(val, unicode):  # noqa: F821
     val = val.encode('utf-8', errors='replace')
   return val
 
@@ -478,7 +497,7 @@ class ImpalaClient(object):
     if self.use_ssl:
       # TSSLSocket needs the ssl module, which may not be standard on all Operating
       # Systems. Only attempt to import TSSLSocket if the user wants an SSL connection.
-      from TSSLSocketWithWildcardSAN import TSSLSocketWithWildcardSAN
+      from impala_shell.TSSLSocketWithWildcardSAN import TSSLSocketWithWildcardSAN
 
     # The kerberos_host_fqdn option exposes the SASL client's hostname attribute to
     # the user. impala-shell checks to ensure this host matches the host in the kerberos
@@ -1468,6 +1487,7 @@ class ImpalaBeeswaxClient(ImpalaClient):
       # shell being installed as a standalone python package from public PyPI,
       # rather than being included as part of a typical Impala deployment.
       #
+      # TODO: Revisit the following:
       # Essentially, it's a hack that is required due to issues stemming from
       # IMPALA-6808. Because of the way the Impala python environment has been
       # somewhat haphazardly constructed, we end up polluting the top level Impala
@@ -1508,7 +1528,7 @@ def log_exception_with_timestamp(e, type="Exception", msg="", stderr_flag=True):
   # method log_exception_with_timestamp prints timestamp with exception trace
   # and accepts custom message before timestamp. stderr_flag controls print statement
   # to be logged in stderr, by default it is true.
-  if(stderr_flag):
+  if (stderr_flag):
       print("%s [%s] %s" % (time.strftime("%Y-%m-%d %H:%M:%S", time.localtime()), type,
       msg), e, file=sys.stderr)
   else:

shell/impala_shell/impala_shell.py

@@ -19,40 +19,54 @@
 # under the License.
 #
 # Impala's shell
-from __future__ import print_function, unicode_literals
-from compatibility import _xrange as xrange
-
+from __future__ import absolute_import, print_function, unicode_literals
 import cmd
 import errno
 import getpass
 import logging
 import os
-import prettytable
 import random
 import re
 import shlex
 import signal
 import socket
-import sqlparse
 import subprocess
+from subprocess import call
 import sys
 import textwrap
 import time
 import traceback
 
-from impala_client import ImpalaHS2Client, StrictHS2Client, \
-    ImpalaBeeswaxClient, QueryOptionLevels, log_exception_with_timestamp, log_timestamp
-from impala_shell_config_defaults import impala_shell_defaults
-from option_parser import get_option_parser, get_config_from_file
-from shell_output import (DelimitedOutputFormatter, OutputStream, PrettyOutputFormatter,
-                          OverwritingStdErrOutputStream, VerticalOutputFormatter,
-                          match_string_type)
-from subprocess import call
-from shell_exceptions import (RPCException, DisconnectedException, QueryStateException,
-    QueryCancelledByShellException, MissingThriftMethodException)
-
-from value_converter import HS2ValueConverter
+import prettytable
+import sqlparse
 
+from impala_shell.compatibility import _xrange as xrange
+from impala_shell.impala_client import (
+    ImpalaBeeswaxClient,
+    ImpalaHS2Client,
+    log_exception_with_timestamp,
+    log_timestamp,
+    QueryOptionLevels,
+    StrictHS2Client,
+)
+from impala_shell.impala_shell_config_defaults import impala_shell_defaults
+from impala_shell.option_parser import get_config_from_file, get_option_parser
+from impala_shell.shell_exceptions import (
+    DisconnectedException,
+    MissingThriftMethodException,
+    QueryCancelledByShellException,
+    QueryStateException,
+    RPCException,
+)
+from impala_shell.shell_output import (
+    DelimitedOutputFormatter,
+    match_string_type,
+    OutputStream,
+    OverwritingStdErrOutputStream,
+    PrettyOutputFormatter,
+    VerticalOutputFormatter,
+)
+from impala_shell.value_converter import HS2ValueConverter
 
 VERSION_FORMAT = "Impala Shell v%(version)s (%(git_hash)s) built on %(build_date)s"
 VERSION_STRING = "impala shell build version not available"
@@ -60,8 +74,10 @@ READLINE_UNAVAILABLE_ERROR = "The readline module was either not found or disabl
                              "Command history will not be collected."
 
 # Tarball / packaging build makes impala_build_version available
+# TODO: There's no reason for this to fail when everything is built around pip installs,
+# so this could be simplified.
 try:
-  from impala_build_version import get_git_hash, get_build_date, get_version
+  from impala_shell.impala_build_version import get_build_date, get_git_hash, get_version
   VERSION_STRING = VERSION_FORMAT % {'version': get_version(),
                                      'git_hash': get_git_hash()[:7],
                                      'build_date': get_build_date()}
@@ -1511,7 +1527,7 @@ class ImpalaShell(cmd.Cmd, object):
       # Python2 will implicitly convert unicode to str when printing to stderr. It's done
       # using the default 'ascii' encoding, which will fail for UTF-8 error messages.
       # Here we use 'utf-8' to explicitly convert 'msg' to str if it's in unicode type.
-      if sys.version_info.major == 2 and isinstance(msg, unicode):
+      if sys.version_info.major == 2 and isinstance(msg, unicode):  # noqa: F821
         msg = msg.encode('utf-8')
       log_exception_with_timestamp(msg)
     except DisconnectedException as e:
@@ -2268,7 +2284,7 @@ def impala_shell_main():
         print(("-k requires a valid kerberos ticket but no valid kerberos "
                "ticket found."), file=sys.stderr)
         raise FatalShellException()
-    except OSError as e:
+    except OSError:
       print('klist not found on the system, install kerberos clients', file=sys.stderr)
       raise FatalShellException()
   elif options.use_ldap:
@@ -2311,7 +2327,7 @@ def impala_shell_main():
 
   if options.verbose:
     try:
-      import thrift.protocol.fastbinary
+      import thrift.protocol.fastbinary  # noqa: F401
     except Exception as e:
       print("WARNING: Failed to load Thrift's fastbinary module. Thrift's "
             "BinaryProtocol will not be accelerated, which can reduce performance. "

shell/impala_shell/impala_shell_config_defaults.py

@@ -20,7 +20,6 @@
 
 # default options used by the Impala shell stored in a dict
 from __future__ import print_function, unicode_literals
-
 import getpass
 import os
 import socket

shell/impala_shell/option_parser.py

@@ -27,8 +27,7 @@
 # [impala.query_options]
 # EXPLAIN_LEVEL=2
 # MT_DOP=2
-from __future__ import print_function, unicode_literals
-
+from __future__ import absolute_import, print_function, unicode_literals
 import sys
 
 try:
@@ -36,9 +35,10 @@ try:
 except ImportError:
   from ConfigParser import ConfigParser  # python2
 
-from impala_shell_config_defaults import impala_shell_defaults
 from optparse import OptionParser, SUPPRESS_HELP
 
+from impala_shell.impala_shell_config_defaults import impala_shell_defaults
+
 
 class ConfigFileFormatError(Exception):
   """Raised when the config file cannot be read by ConfigParser."""
@@ -383,12 +383,12 @@ def get_option_parser(defaults):
       if short_opt in defaults:
         if option.dest not in defaults:
           defaults[option.dest] = defaults[short_opt]
-        elif type(defaults[option.dest]) == list:
+        elif isinstance(defaults[option.dest], list):
           defaults[option.dest].extend(defaults[short_opt])
       elif long_opt in defaults:
         if option.dest not in defaults:
           defaults[option.dest] = defaults[long_opt]
-        elif type(defaults[option.dest]) == list:
+        elif isinstance(defaults[option.dest], list):
           defaults[option.dest].extend(defaults[long_opt])
 
     # since the quiet flag is the same as the verbose flag

shell/impala_shell/shell_output.py

@@ -17,8 +17,7 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-from __future__ import print_function, unicode_literals
-
+from __future__ import absolute_import, print_function, unicode_literals
 import csv
 import re
 import sys
@@ -38,11 +37,11 @@ def match_string_type(str_to_convert, reference_str):
     assert isinstance(str_to_convert, str)
     return str_to_convert
 
-  if type(str_to_convert) == type(reference_str):
+  if isinstance(str_to_convert, type(reference_str)):
     return str_to_convert
 
   if isinstance(reference_str, str):
-    assert isinstance(str_to_convert, unicode)
+    assert isinstance(str_to_convert, unicode)  # noqa: F821
     return str_to_convert.encode('UTF-8')
   else:
     assert isinstance(reference_str, str)
@@ -103,8 +102,8 @@ class DelimitedOutputFormatter(object):
                         lineterminator='\n', quoting=csv.QUOTE_MINIMAL)
     for row in rows:
       if sys.version_info.major == 2:
-        row = [val.encode('utf-8', 'replace') if isinstance(val, unicode) else val
-            for val in row]
+        row = [val.encode('utf-8', 'replace') if isinstance(val, unicode)  # noqa: F821
+            else val for val in row]
       writer.writerow(row)
     # The CSV writer produces an extra newline. Strip that extra newline (and
     # only that extra newline). csv wraps newlines for data values in quotes,
@@ -132,10 +131,10 @@ class VerticalOutputFormatter(DelimitedOutputFormatter):
                         lineterminator='\n', quoting=csv.QUOTE_MINIMAL)
     for r, row in enumerate(rows):
       if sys.version_info.major == 2:
-        row = [val.encode('utf-8', 'replace') if isinstance(val, unicode) else val
-            for val in row]
-      writer.writerow(["************************************** " +
-        str(r + 1) + ".row **************************************"])
+        row = [val.encode('utf-8', 'replace') if isinstance(val, unicode)  # noqa: F821
+            else val for val in row]
+      writer.writerow(["************************************** "
+        + str(r + 1) + ".row **************************************"])
       for c, val in enumerate(row):
         row[c] = self.column_names[c].rjust(self.column_name_max_len) + ": " + val
       writer.writerow(row)

shell/impala_shell/value_converter.py

@@ -15,10 +15,11 @@
 # specific language governing permissions and limitations
 # under the License.
 
-from impala_thrift_gen.TCLIService.TCLIService import TTypeId
-
+from __future__ import absolute_import
 import sys
 
+from impala_thrift_gen.TCLIService.TCLIService import TTypeId
+
 
 class ValueConverter(object):
 

shell/packaging/__init__.py

@@ -1,40 +0,0 @@
-#!/usr/bin/env python
-#
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#   http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing,
-# software distributed under the License is distributed on an
-# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-# KIND, either express or implied.  See the License for the
-# specific language governing permissions and limitations
-# under the License.
-from os.path import dirname, abspath
-import sys
-
-# When installing the python shell as a standalone package, this __init__ is
-# used to workaround the issues stemming from IMPALA-6808. Because of the way
-# the Impala python environment has been somewhat haphazardly constructed in
-# a deployed cluster, it ends up being "polluted" with top-level modules that
-# should really be sub-modules. One of the principal places this occurs is with
-# the various modules required by the Impala shell. This isn't a concern when
-# the shell is invoked via a specially installed version of python that belongs
-# to Impala, but it does become an issue when the shell is being run using the
-# system python.
-#
-# If we want to install the shell as a standalone package, we need to construct
-# it in such a way that all of the internal modules are contained within a
-# top-level impala_shell namespace. However, this then breaks various imports
-# throughout the Impala shell code. The way this file corrects that is to add
-# the impala_shell directory to PYTHONPATH only when the shell is invoked. As
-# far as I can tell, there's no cleaner way to address this without fully
-# resolving IMPALA-6808.
-impala_shell_dir = dirname(abspath(__file__))
-sys.path.append(impala_shell_dir)

shell/packaging/make_python_package.sh

@@ -24,11 +24,10 @@
 #
 #   ${DIST_DIR}/impala_shell-<version>.tar.gz
 #
-# Until the thrift-generated python files in ${IMPALA_HOME}/shell/gen-py
+# Until the thrift-generated python files in ${IMPALA_HOME}/shell/impala_thrift_gen
 # have been created by the build process, this script will not work.
 # It also relies upon the impala_build_version.py file created by the
-# ${IMPALA_HOME}/shell/gen_impala_build_version.sh script, which needs
-# to run before this script will work.
+# ${IMPALA_HOME}/shell/gen_impala_build_version.sh script.
 #
 # After those files exist, however, this script can be run again at will.
 
@@ -39,36 +38,18 @@ SHELL_HOME="${IMPALA_HOME}"/shell
 STAGING_DIR="${WORKING_DIR}"/staging
 DIST_DIR="${DIST_DIR:-$SHELL_HOME/dist}"
 PACKAGE_DIR="${STAGING_DIR}"/impala_shell_package
-MODULE_LIB_DIR="${PACKAGE_DIR}"/impala_shell
 NO_CLEAN_DIST="${NO_CLEAN_DIST:-}"
 
-THRIFT_GEN_PY_DIR=${SHELL_HOME}/gen-py
-
 assemble_package_files() {
-  mkdir -p "${MODULE_LIB_DIR}"
+  mkdir -p "${PACKAGE_DIR}"
 
-  cp -r "${THRIFT_GEN_PY_DIR}"/* "${MODULE_LIB_DIR}"
+  cp -r "${SHELL_HOME}/impala_thrift_gen" "${PACKAGE_DIR}"
+  cp -r "${SHELL_HOME}/impala_shell" "${PACKAGE_DIR}"
 
-  cp "${WORKING_DIR}/__init__.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/compatibility.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/impala_shell.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/impala_client.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/option_parser.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/shell_output.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/impala_shell_config_defaults.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/TSSLSocketWithWildcardSAN.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/ImpalaHttpClient.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/shell_exceptions.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/cookie_util.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/kerberos_util.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/value_converter.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/thrift_printer.py" "${MODULE_LIB_DIR}"
-  cp "${SHELL_HOME}/exec_summary.py" "${MODULE_LIB_DIR}"
-
-  cp "${SHELL_HOME}/packaging/README.md" "${PACKAGE_DIR}"
-  cp "${SHELL_HOME}/packaging/MANIFEST.in" "${PACKAGE_DIR}"
-  cp "${SHELL_HOME}/packaging/requirements.txt" "${PACKAGE_DIR}"
-  cp "${SHELL_HOME}/packaging/setup.py" "${PACKAGE_DIR}"
+  cp "${SHELL_HOME}/README.md" "${PACKAGE_DIR}"
+  cp "${SHELL_HOME}/MANIFEST.in" "${PACKAGE_DIR}"
+  cp "${SHELL_HOME}/requirements.txt" "${PACKAGE_DIR}"
+  cp "${SHELL_HOME}/setup.py" "${PACKAGE_DIR}"
 
   cp "${IMPALA_HOME}/LICENSE.txt" "${PACKAGE_DIR}"
 }

shell/packaging/make_shell_tarball.sh

@@ -71,7 +71,7 @@ for PYTHON_EXE in $*; do
 done
 
 # Copy the impala-shell driver script into the tarball root
-cp ${SHELL_HOME}/impala-shell ${TARBALL_ROOT}
+cp ${SHELL_HOME}/packaging/impala-shell ${TARBALL_ROOT}
 
 pushd ${BUILD_DIR} > /dev/null
 echo "Making tarball in ${BUILD_DIR}"

tests/beeswax/impala_beeswax.py

@@ -258,7 +258,7 @@ class ImpalaBeeswaxClient(object):
     return output
 
   def __build_summary_table(self, summary, output):
-    from shell.exec_summary import build_exec_summary_table
+    from impala_shell.exec_summary import build_exec_summary_table
     result = list()
     build_exec_summary_table(summary, 0, 0, False, result, is_prettyprint=False,
                              separate_prefix_column=True)

tests/common/impala_connection.py

@@ -28,10 +28,10 @@ import re
 import time
 
 from future.utils import with_metaclass
-
 import impala.dbapi as impyla
 import impala.error as impyla_error
 import impala.hiveserver2 as hs2
+
 from impala_thrift_gen.beeswax.BeeswaxService import QueryState
 from impala_thrift_gen.RuntimeProfile.ttypes import TRuntimeProfileFormat
 from tests.beeswax.impala_beeswax import (
@@ -112,7 +112,7 @@ def format_sql_for_logging(sql_stmt):
 
 
 def build_summary_table_from_thrift(thrift_exec_summary):
-  from shell.exec_summary import build_exec_summary_table
+  from impala_shell.exec_summary import build_exec_summary_table
   result = list()
   build_exec_summary_table(thrift_exec_summary, 0, 0, False, result,
                            is_prettyprint=False, separate_prefix_column=True)

tests/custom_cluster/test_hs2_fault_injection.py

@@ -16,16 +16,17 @@
 # under the License.
 
 from __future__ import absolute_import, division, print_function
+from time import sleep
+
 from builtins import round
 import pytest
 import requests
 
-from shell.ImpalaHttpClient import ImpalaHttpClient
-from shell.impala_client import ImpalaHS2Client
-from shell.shell_exceptions import HttpError
-from tests.common.impala_test_suite import IMPALAD_HS2_HTTP_HOST_PORT
+from impala_shell.impala_client import ImpalaHS2Client
+from impala_shell.ImpalaHttpClient import ImpalaHttpClient
+from impala_shell.shell_exceptions import HttpError
 from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
-from time import sleep
+from tests.common.impala_test_suite import IMPALAD_HS2_HTTP_HOST_PORT
 
 """IMPALA-12216 implemented timestamp to be printed in case of any error/warning
   during query execution, below is an example :
@@ -147,33 +148,33 @@ class TestHS2FaultInjection(CustomClusterTestSuite):
 
   def __expect_msg_retry(self, impala_rpc_name):
     """Returns expected log message for rpcs which can be retried"""
-    return ("[Exception] type=<class 'shell.shell_exceptions.HttpError'> in {0}. "
+    return ("[Exception] type=<class 'impala_shell.shell_exceptions.HttpError'> in {0}. "
       "Num remaining tries: 3 HTTP code 502: Injected Fault".format(impala_rpc_name))
 
   def __expect_msg_retry_with_extra(self, impala_rpc_name):
     """Returns expected log message for rpcs which can be retried and where the http
     message has a message body"""
-    return ("[Exception] type=<class 'shell.shell_exceptions.HttpError'> in {0}. "
+    return ("[Exception] type=<class 'impala_shell.shell_exceptions.HttpError'> in {0}. "
       "Num remaining tries: 3 HTTP code 503: Injected Fault [EXTRA]"
       .format(impala_rpc_name))
 
   def __expect_msg_retry_with_retry_after(self, impala_rpc_name):
     """Returns expected log message for rpcs which can be retried and the http
     message has a body and a Retry-After header that can be correctly decoded"""
-    return ("[Exception] type=<class 'shell.shell_exceptions.HttpError'> in {0}. "
+    return ("[Exception] type=<class 'impala_shell.shell_exceptions.HttpError'> in {0}. "
       "Num remaining tries: 3, retry after 1 secs "
       "HTTP code 503: Injected Fault [EXTRA]".format(impala_rpc_name))
 
   def __expect_msg_retry_with_retry_after_no_extra(self, impala_rpc_name):
     """Returns expected log message for rpcs which can be retried and the http
     message has a Retry-After header that can be correctly decoded"""
-    return ("[Exception] type=<class 'shell.shell_exceptions.HttpError'> in {0}. "
+    return ("[Exception] type=<class 'impala_shell.shell_exceptions.HttpError'> in {0}. "
       "Num remaining tries: 3, retry after 1 secs "
       "HTTP code 503: Injected Fault".format(impala_rpc_name))
 
   def __expect_msg_no_retry(self, impala_rpc_name):
     """Returns expected log message for rpcs which can not be retried"""
-    return ("[Exception] type=<class 'shell.shell_exceptions.HttpError'> in {0}.  "
+    return ("[Exception] type=<class 'impala_shell.shell_exceptions.HttpError'> in {0}.  "
       "HTTP code 502: Injected Fault".format(impala_rpc_name))
 
   @pytest.mark.execute_serially

tests/custom_cluster/test_shell_interactive_reconnect.py

@@ -16,19 +16,18 @@
 # under the License.
 
 from __future__ import absolute_import, division, print_function
-import pytest
-import tempfile
 import socket
-import pexpect
-import os
 
+import pexpect
+import pytest
+
+# Follow tests/shell/test_shell_interactive.py naming.
+from impala_shell.impala_shell import ImpalaShell as ImpalaShellClass
 from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
 from tests.common.impala_service import ImpaladService
-from tests.common.test_vector import ImpalaTestVector
 from tests.common.test_dimensions import create_client_protocol_dimension
-from tests.shell.util import ImpalaShell, get_shell_cmd, get_impalad_port, spawn_shell
-# Follow tests/shell/test_shell_interactive.py naming.
-from shell.impala_shell import ImpalaShell as ImpalaShellClass
+from tests.common.test_vector import ImpalaTestVector
+from tests.shell.util import get_impalad_port, get_shell_cmd, ImpalaShell, spawn_shell
 from tests.verifiers.metric_verifier import MetricVerifier
 
 NUM_QUERIES = 'impala-server.num-queries'

tests/custom_cluster/test_thrift_socket.py

@@ -17,21 +17,21 @@
 
 from __future__ import absolute_import, division, print_function
 import os
-import pytest
 import ssl
 import sys
 import time
 
+import pytest
+
 # 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.environ import IS_REDHAT_DERIVATIVE
+from impala_shell.impala_shell import ImpalaShell as ImpalaShellClass
 from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
-from tests.common.test_vector import ImpalaTestVector
+from tests.common.environ import IS_REDHAT_DERIVATIVE
 from tests.common.test_dimensions import create_client_protocol_dimension
+from tests.common.test_vector import ImpalaTestVector
 from tests.shell.util import ImpalaShell
 
 REQUIRED_MIN_OPENSSL_VERSION = 0x10001000

tests/shell/test_cookie_util.py

@@ -19,16 +19,16 @@
 # under the License.
 
 from __future__ import absolute_import, division, print_function
-
-import sys
-
-from tests.common.base_test_suite import BaseTestSuite
-
 from datetime import datetime, timedelta
 from http.client import HTTPMessage
+import sys
 
-from shell.cookie_util import (cookie_matches_path, get_cookie_expiry,
-                               get_all_matching_cookies)
+from impala_shell.cookie_util import (
+    cookie_matches_path,
+    get_all_matching_cookies,
+    get_cookie_expiry,
+)
+from tests.common.base_test_suite import BaseTestSuite
 
 
 class TestCookieUtil(BaseTestSuite):

tests/shell/test_kerberos_util.py

@@ -20,7 +20,7 @@
 
 from __future__ import absolute_import, division, print_function
 
-from shell.kerberos_util import get_kerb_host_from_kerberos_host_fqdn
+from impala_shell.kerberos_util import get_kerb_host_from_kerberos_host_fqdn
 from tests.common.base_test_suite import BaseTestSuite
 
 

tests/shell/test_shell_client.py

@@ -19,11 +19,15 @@
 # under the License.
 
 from __future__ import absolute_import, division, print_function
-from shell.impala_client import ImpalaBeeswaxClient, ImpalaHS2Client
+
+from impala_shell.impala_client import ImpalaBeeswaxClient, ImpalaHS2Client
 from tests.common.impala_test_suite import ImpalaTestSuite
 from tests.common.test_dimensions import (
-  create_client_protocol_dimension, create_client_protocol_no_strict_dimension,
-  create_uncompressed_text_dimension, create_single_exec_option_dimension)
+    create_client_protocol_dimension,
+    create_client_protocol_no_strict_dimension,
+    create_single_exec_option_dimension,
+    create_uncompressed_text_dimension,
+)
 from tests.shell.util import get_impalad_host_port
 
 

tests/shell/test_shell_commandline.py

@@ -19,34 +19,43 @@
 # under the License.
 
 from __future__ import absolute_import, division, print_function
-from builtins import range
+from contextlib import closing
 import errno
 import getpass
 import os
-import pytest
 import re
 import signal
 import socket
-import tempfile
-
-from shell.impala_shell import ImpalaShell as ImpalaShellClass
-
 from subprocess import call, Popen
+import tempfile
+from time import sleep, time
+
+from builtins import range
+import pytest
+
+from impala_shell.impala_shell import ImpalaShell as ImpalaShellClass
 from tests.common.environ import ImpalaTestClusterProperties
 from tests.common.impala_service import ImpaladService
-from tests.common.impala_test_suite import ImpalaTestSuite, IMPALAD_HS2_HOST_PORT
+from tests.common.impala_test_suite import IMPALAD_HS2_HOST_PORT, ImpalaTestSuite
 from tests.common.skip import SkipIf
 from tests.common.test_dimensions import (
-  create_client_protocol_dimension, create_client_protocol_strict_dimension,
-  create_uncompressed_text_dimension, create_single_exec_option_dimension)
+    create_client_protocol_dimension,
+    create_client_protocol_strict_dimension,
+    create_single_exec_option_dimension,
+    create_uncompressed_text_dimension,
+)
 from tests.common.test_result_verifier import error_msg_startswith
-from time import sleep, time
-from tests.shell.util import (get_impalad_host_port, assert_var_substitution,
-  run_impala_shell_cmd, ImpalaShell, build_shell_env, wait_for_query_state,
-  create_impala_shell_executable_dimension, get_impala_shell_executable,
-  stderr_get_first_error_msg)
-from contextlib import closing
-
+from tests.shell.util import (
+    assert_var_substitution,
+    build_shell_env,
+    create_impala_shell_executable_dimension,
+    get_impala_shell_executable,
+    get_impalad_host_port,
+    ImpalaShell,
+    run_impala_shell_cmd,
+    stderr_get_first_error_msg,
+    wait_for_query_state,
+)
 
 DEFAULT_QUERY = 'select 1'
 QUERY_FILE_PATH = os.path.join(os.environ['IMPALA_HOME'], 'tests', 'shell')

tests/shell/test_shell_interactive.py

@@ -23,34 +23,46 @@ import http.client
 import http.server
 import logging
 import os
-import pexpect
-import pytest
 import re
 import signal
 import socket
 import socketserver
 import sys
+from tempfile import NamedTemporaryFile
 import threading
 from time import sleep
 
+import pexpect
+import pytest
+
 # 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 tempfile import NamedTemporaryFile
+from impala_shell.impala_shell import ImpalaShell as ImpalaShellClass
+from impala_shell.impala_shell import TIPS
 from tests.common.impala_service import ImpaladService
 from tests.common.impala_test_suite import ImpalaTestSuite
 from tests.common.skip import SkipIfLocal
 from tests.common.test_dimensions import (
-  create_client_protocol_dimension, create_client_protocol_strict_dimension,
-  create_uncompressed_text_dimension, create_single_exec_option_dimension)
+    create_client_protocol_dimension,
+    create_client_protocol_strict_dimension,
+    create_single_exec_option_dimension,
+    create_uncompressed_text_dimension,
+)
 from tests.common.test_result_verifier import error_msg_startswith
-from tests.shell.util import (assert_var_substitution, ImpalaShell, get_impalad_port,
-  get_shell_cmd, get_open_sessions_metric, spawn_shell, get_unused_port,
-  create_impala_shell_executable_dimension, get_impala_shell_executable,
-  stderr_get_first_error_msg)
+from tests.shell.util import (
+    assert_var_substitution,
+    create_impala_shell_executable_dimension,
+    get_impala_shell_executable,
+    get_impalad_port,
+    get_open_sessions_metric,
+    get_shell_cmd,
+    get_unused_port,
+    ImpalaShell,
+    spawn_shell,
+    stderr_get_first_error_msg,
+)
 
 QUERY_FILE_PATH = os.path.join(os.environ['IMPALA_HOME'], 'tests', 'shell')
 
@@ -707,14 +719,8 @@ class TestImpalaShellInteractive(ImpalaTestSuite):
 
   def test_tip(self, vector):
     """Smoke test for the TIP command"""
-    # Temporarily add impala_shell module to path to get at TIPS list for verification
-    sys.path.append("%s/shell/" % os.environ['IMPALA_HOME'])
-    try:
-      import impala_shell
-    finally:
-      sys.path = sys.path[:-1]
     result = run_impala_shell_interactive(vector, "tip;")
-    for t in impala_shell.TIPS:
+    for t in TIPS:
       if t in result.stderr: return
     assert False, "No tip found in output %s" % result.stderr
 

tests/shell/util.py

@@ -19,31 +19,33 @@
 # under the License.
 
 from __future__ import absolute_import, division, print_function
+from contextlib import closing
 import logging
 import os
+import re
+import shlex
 import socket
-from contextlib import closing
+from subprocess import PIPE, Popen
+import sys
+import time
 
 import pexpect
 import pytest
-import re
-import shlex
-import sys
-import time
-from subprocess import Popen, PIPE
 
 # 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.environ import (IMPALA_LOCAL_BUILD_VERSION,
-                                  ImpalaTestClusterProperties)
+from impala_shell.impala_shell import ImpalaShell as ImpalaShellClass
+from tests.common.environ import IMPALA_LOCAL_BUILD_VERSION, ImpalaTestClusterProperties
 from tests.common.impala_service import ImpaladService
-from tests.common.impala_test_suite import (IMPALAD_BEESWAX_HOST_PORT,
-    IMPALAD_HS2_HOST_PORT, IMPALAD_HS2_HTTP_HOST_PORT,
-    STRICT_HS2_HOST_PORT, STRICT_HS2_HTTP_HOST_PORT)
+from tests.common.impala_test_suite import (
+    IMPALAD_BEESWAX_HOST_PORT,
+    IMPALAD_HS2_HOST_PORT,
+    IMPALAD_HS2_HTTP_HOST_PORT,
+    STRICT_HS2_HOST_PORT,
+    STRICT_HS2_HTTP_HOST_PORT,
+)
 from tests.common.test_vector import ImpalaTestDimension
 
 LOG = logging.getLogger('tests/shell/util.py')