cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andy Tolbert (Jira)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-10190) Python 3 support for cqlsh
Date Mon, 07 Oct 2019 04:38:00 GMT

    [ 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: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org


Mime
View raw message