[
https://issues.apache.org/jira/browse/CASSANDRA-10190?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16945585#comment-16945585
]
Andy Tolbert commented on CASSANDRA-10190:
------------------------------------------
Hi [~ptbannister], this is much needed and great work! I took a look at your
branch today and it looks really good to me. I have some minor suggestions, but
I think things are pretty good as is!
I made patches of my suggestions below ([^0001-Update-six-to-1.12.0.patch],
[^0002-Simplify-version-specific-logic-by-using-six.moves-a.patch]). Note that
other than the import change in {{winpty.py}}, these changes don't really fix
anything, so I would not mind if you chose not to accept them.
Recommended changes:
{{six}}
* I realized that a very old version of six (1.7.3) is being provided via lib
as the cassandra-driver being pulled in claims to require six 1.9.0. We should
consider upgrading that, especially since there are some newer features
(covered below) that may be useful.
* There are some additional features of six that could be used which would
simplify/remove python version checking logic. For example, there's a lot of
code that calls encode/decode if PY2 is used, {{six.ensure_text}} could be used
instead.
{{bin/cqlsh}}
* L85-95: Commented out code probably not needed anymore?
{{bin/cqlsh.py}}
* L50-56: Could make use of
[six.moves|https://six.readthedocs.io/#module-six.moves] to impor in a
version-agnostic way, i.e. from {{six.moves import configparser}}, {{from
six.moves import cStringIO as StringIO}})
* L867: Instead of {{_input}}, could use {{six.moves.input}}.
{{pylib/copyutil.py}}
* L48-54: use {{six.moves}} (see comment ^ {{cqlsh.py}})
* where {{range}} is used in favor of {{xrange}}, it would be good to use
{{six.moves.range}} instead, since when using python2 {{xrange}} will be used
so the existing behavior for python2 will be maintained.
{{pylib/cqlshlib/formatting.py}}
* {{ensure_text}} can be utilized in a places to simplify conversion logic.
{{pylib/cqlshlib/saferscanner.py}}
* Unless i'm mistaken {{Py35SaferScanner}} and {{Py36SaferScanner}} are the
same, {{Py35SaferScanner}} not used, so can be removed.
{{pylib/cqlshlib/sslhandling.py}}
* L21-24, use {{six.moves}}
{{pylib/cqlshlib/test/winpty.py}} & {{pylib/cqlshlib/util.py}}
* import queue won't work with python2, & might as well use cStringIO
The tests all passed for me except the one you mentioned with python3
(TestCqlshOutput#test_user_types). I’m pretty certain this is fixed in newer
python driver versions. The driver bundled with C* is several minor versions
old and is an internal build with some changes for transient replicas (I’m
guessing has to do with the way you express replication when creating a
keyspace with transient replicas). I think it's worth updating the driver to a
newer version, and maybe that can be done in a separate patch unless you think
it should be done here?
> Python 3 support for cqlsh
> --------------------------
>
> Key: CASSANDRA-10190
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10190
> Project: Cassandra
> Issue Type: Improvement
> Components: Legacy/Tools
> Reporter: Andrew Pennebaker
> Assignee: Patrick Bannister
> Priority: Normal
> Labels: cqlsh
> Fix For: 4.0, 4.0-alpha
>
> Attachments: 0001-Update-six-to-1.12.0.patch,
> 0002-Simplify-version-specific-logic-by-using-six.moves-a.patch,
> coverage_notes.txt
>
>
> Users who operate in a Python 3 environment may have trouble launching cqlsh.
> Could we please update cqlsh's syntax to run in Python 3?
> As a workaround, users can setup pyenv, and cd to a directory with a
> .python-version containing "2.7". But it would be nice if cqlsh supported
> modern Python versions out of the box.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
