This is the main patch for making the the impala-shell cross-compatible with
python 2 and python 3. The goal is wind up with a version of the shell that will
pass python e2e tests irrepsective of the version of python used to launch the
shell, under the assumption that the test framework itself will continue to run
with python 2.7.x for the time being.
Notable changes for reviewers to consider:
- With regard to validating the patch, my assumption is that simply passing
the existing set of e2e shell tests is sufficient to confirm that the shell
is functioning properly. No new tests were added.
- A new pytest command line option was added in conftest.py to enable a user
to specify a path to an alternate impala-shell executable to test. It's
possible to use this to point to an instance of the impala-shell that was
installed as a standalone python package in a separate virtualenv.
Example usage:
USE_THRIFT11_GEN_PY=true impala-py.test --shell_executable=/<path to virtualenv>/bin/impala-shell -sv shell/test_shell_commandline.py
The target virtualenv may be based on either python3 or python2. However,
this has no effect on the version of python used to run the test framework,
which remains tied to python 2.7.x for the foreseeable future.
- The $IMPALA_HOME/bin/impala-shell.sh now sets up the impala-shell python
environment independenty from bin/set-pythonpath.sh. The default version
of thrift is thrift-0.11.0 (See IMPALA-9489).
- The wording of the header changed a bit to include the python version
used to run the shell.
Starting Impala Shell with no authentication using Python 3.7.5
Opened TCP connection to localhost:21000
...
OR
Starting Impala Shell with LDAP-based authentication using Python 2.7.12
Opened TCP connection to localhost:21000
...
- By far, the biggest hassle has been juggling str versus unicode versus
bytes data types. Python 2.x was fairly loose and inconsistent in
how it dealt with strings. As a quick demo of what I mean:
Python 2.7.12 (default, Nov 12 2018, 14:36:49)
[GCC 5.4.0 20160609] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> d = 'like a duck'
>>> d == str(d) == bytes(d) == unicode(d) == d.encode('utf-8') == d.decode('utf-8')
True
...and yet there are weird unexpected gotchas.
>>> d.decode('utf-8') == d.encode('utf-8')
True
>>> d.encode('utf-8') == bytearray(d, 'utf-8')
True
>>> d.decode('utf-8') == bytearray(d, 'utf-8') # fails the eq property?
False
As a result, this was inconsistency was reflected in the way we handled
strings in the impala-shell code, but things still just worked.
In python3, there's a much clearer distinction between strings and bytes, and
as such, much tighter type consistency is expected by standard libs like
subprocess, re, sqlparse, prettytable, etc., which are used throughout the
shell. Even simple calls that worked in python 2.x:
>>> import re
>>> re.findall('foo', b'foobar')
['foo']
...can throw exceptions in python 3.x:
>>> import re
>>> re.findall('foo', b'foobar')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/data0/systest/venvs/py3/lib/python3.7/re.py", line 223, in findall
return _compile(pattern, flags).findall(string)
TypeError: cannot use a string pattern on a bytes-like object
Exceptions like this resulted in a many, if not most shell tests failing
under python 3.
What ultimately seemed like a better approach was to try to weed out as many
existing spurious str.encode() and str.decode() calls as I could, and try to
implement what is has colloquially been called a "unicode sandwich" -- namely,
"bytes on the outside, unicode on the inside, encode/decode at the edges."
The primary spot in the shell where we call decode() now is when sanitising
input...
args = self.sanitise_input(args.decode('utf-8'))
...and also whenever a library like re required it. Similarly, str.encode()
is primarily used where a library like readline or csv requires is.
- PYTHONIOENCODING needs to be set to utf-8 to override the default setting for
python 2. Without this, piping or redirecting stdout results in unicode errors.
- from __future__ import unicode_literals was added throughout
Testing:
To test the changes, I ran the e2e shell tests the way we always do (against
the normal build tarball), and then I set up a python 3 virtual env with the
shell installed as a package, and manually ran the tests against that.
No effort has been made at this point to come up with a way to integrate
testing of the shell in a python3 environment into our automated test
processes.
Change-Id: Idb004d352fe230a890a6b6356496ba76c2fab615
Reviewed-on: http://gerrit.cloudera.org:8080/15524
Reviewed-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
In an effort to keep the work of reviewing the changes more manageable
with regard to making the impala-shell python3 compatible, I'm trying
to break the patches up into smaller chunks.
The first patch is the easiest one -- simply addressing the handful of
syntax issues that aren't python 3 compatible, namely changing the
print statements to function calls, changing the way we catch exceptions,
and adding a few simple branches to work around the removal of such
things as dict.iteritems().
We needed the print function imported from __future__ because it allows
us to pass in a file descriptor, e.g., sys.stderr.
Notably, there's nothing in this patch related to string/bytes/unicode
changes from python 2 to 3.
Change-Id: I9a515da01ef03d5936cb1a4d9e4bc6d105386b1d
Reviewed-on: http://gerrit.cloudera.org:8080/15487
Reviewed-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
IMPALA-5690 replaced thrift 0.9.0 with 0.9.3 in which THRIFT-3505
changed transport/TSSLSocket.py.
In thrift 0.9.3, if the python version is lower than 2.7.9, TSSLSocket
uses PROTOCOL_TLSv1 by default and the SSL version is passed to
TSSLSocket as a paramter when calling TSSLSocket.__init__.
Although TLSv1.2 is supported by Python from 2.7.9, Red Hat/CentOS
support TLSv1.2 from 2.7.5 with upgraded python-libs. We need to get
impala-shell support TLSv1.2 with Python 2.7.5 on Red Hat/CentOS.
TESTING:
impala-py.test tests/custom_cluster/test_client_ssl.py
Change-Id: I3fb6510f4b556bd8c6b1e86380379aba8be4b805
Reviewed-on: http://gerrit.cloudera.org:8080/13457
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
In Thrift 0.9.3 the TSSLSocket initializer TSSLSocket.__init__ prints
warnings if positional parameters are used. Change our usage of this
initializer to use named parameters.
Follow up work on "IMPALA-8333 Remove Impala Shell warnings part 2" will
remove one further warning message.
TESTING
Ran all end-to-end tests.
Added tests for the deprecation warnings to test_client_ssl.py.
Change-Id: I31f9a0bb12ca6a1da9129eacd29ac105b883e01b
Reviewed-on: http://gerrit.cloudera.org:8080/12837
Reviewed-by: Fredy Wijaya <fwijaya@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
The shell uses Thrift's TSSLSocket to negotiate secure connections to
Impala. This socket uses a variable SSL_VERSION to determine which SSL
and TLS protocol versions it will connect to.
SSL_VERSION was hardcoded to be PROTOCOL_TLSv1, which only supports
TLSv1 servers and no other protocol version. Change the allowed version
to be PROTOCOL_SSLv23, which supports any TLS or SSL protocol. We rely
on the server not to allow SSLv2 or v3 connections.
Testing: Added a new custom cluster test to confirm that the shell can
connect to a TLSv1.2 cluster. Confirmed that the test is correctly
skipped on machines with an old version of OpenSSL that does not support
TLSv1.2.
Change-Id: I5487f82d110676b9c3c7a5305931da00c7f68ca0
Reviewed-on: http://gerrit.cloudera.org:8080/7675
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
This file snuck in with the old copyright header after I put together
the original change for this JIRA.
Change-Id: I311e493bec7e63ea6dd7229140045d486540612a
Reviewed-on: http://gerrit.cloudera.org:8080/3867
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Tested-by: Internal Jenkins
The impala-shell could not accept wildcard or SAN certificates
previously as the thrift library it depended on did not support them.
This patch subclasses TSSLSocket and adds the logic to take care of
the above mentioned cases by introducing the new
TSSLSocketWithWildcardSAN class.
The certificate matching logic is based on the python-ssl source code.
Added custom cluster tests to test both wildcard matching and SAN
matching.
Added be/src/testutil/certificates-info.txt which contains all the
information about the certificates which are added for the tests.
This has been tested with Python2.4 and Python2.6.
Change-Id: I75e37012eeeb0bcf87a5edf875f0ff915daf8b89
Reviewed-on: http://gerrit.cloudera.org:8080/3765
Reviewed-by: Sailesh Mukil <sailesh@cloudera.com>
Tested-by: Internal Jenkins
