Remove some remnants of Python2 #1172

Lorak-mmk · 2023-07-19T18:13:52Z

Support for Python < 3.7 was dropped some time ago, but there were still a lot of places in the codebase with workarounds required for those versions - most notably usage of six package.
This PR removes all usages of six, removes six from dependencies, removes all workarounds for older versions that I found and fixes comments / docs references.
There are 2 commits. First one drops six - it is pretty big, but the changes are mostly very simple and mechanical.
Second one removes other mentions / workaround, unrelated to six.

This PR is a based on scylladb#243 - just rebased it on this repository's master and solved the conflicts.

As we no longer support Python 2, there is no reason to keep this dependency. This commit removes all usages of six and removes it from dependencies.

There are some stale mentions in docs / comments about Python versions that are no longer supported. There are also some workarounds to make driver work with those versions. This commit removes all mentions and workarounds that I was able to find.

fruch · 2023-07-19T21:04:45Z

@Lorak-mmk

#1168 overlaps a bit of this.

Notice that PR updated the CI to use python 3.7, and it's not clear if the python2 support is removed on this repo..

Since the CI still runs python2

Lorak-mmk · 2023-07-19T21:48:21Z

Oh, I didn't see #1168 . My PR goes further anyway, so I don't think that's much of a problem.

Do you know if this appveyor CI is really used here? I see it didn't pass even once for the past 2 years, always fails (I can't see older runs).

absurdfarce · 2023-07-20T05:38:01Z

Thanks for the PR @Lorak-mmk!

Have you signed the Contributor License Agreement for contributions to DataStax open source projects? If not you can find it at https://cla.datastax.com/. Thanks!

Regarding your question about CI: the AppVeyor configuration is out-of-date and can be ignored. What's relevant for purposes of getting any changes merged is the output of the Travis test runs.

Lorak-mmk · 2023-07-20T06:49:09Z

I'm contributing this code on behalf of my employer ScyllaDB. We have signed Contributor License Agreement in the past when we were contributing fixes to Java driver. For more context see apache/cassandra-java-driver#1195

bschoening · 2023-08-03T12:56:56Z

#1168 ran successfully with AppVeyor by removing python 2.7 from the yaml config. You could cherry pick that change and/or add it to 1172.

With the impending freeze of Cassandra 5.x trunk, it would be great to get an updated Python driver w/out 'six' incorporated there. Currently, the driver six dependency cascades into Apache Cassandra and CCM having a six dependency.

Lorak-mmk · 2023-08-03T15:17:24Z

I cherry picked this commit, is it okay with you @vgali7 ?

vgali7 · 2023-08-03T16:01:13Z

@Lorak-mmk Yeah certainly

fruch · 2023-08-29T12:22:19Z

@absurdfarce @bschoening, is there anything else blocking this one from getting merged ?

bschoening · 2023-08-30T14:36:48Z

+1

absurdfarce · 2023-08-30T21:02:41Z

Thanks for the question @fruch ! The only remaining issue with this PR that I'm aware of is that we need to complete a review of it. That work is in process but I'm trying to juggle a lot of priorities at the moment so I haven't had a chance to dig into this in detail yet.

I can say that this change (or, more broadly, removing six from the code base) is one of the things we'd like to include in our next release (likely 3.29.0). So it's very definitely on my radar... it's just a matter of finding time to get it reviewed and merged.

bschoening · 2023-08-31T17:56:05Z

Perhaps we could break this down into bit-sized chunks which can be quickly reviewed?

First, fixing AppVeyor by removing python 2.7 from the yaml config is really useful because it lets builds complete successfully. I could submit a separate PR for just that change.

Second, Vineet's #1168 was designed to be quick to review as it ONLY removes 'if six.PY2' logic (and includes AppVeyor above)

All the above changes in this PR to completely remove six are great, but all together a bit complex to review.

absurdfarce · 2023-10-09T15:45:35Z

Okay, so I now have the availability to give this some time; apologies all for the delay here.

Most of this looks pretty solid to me; thanks @Lorak-mmk and/or @fruch for all the work here (and for contributing upstream). Regarding the AppVeyor build: it's good to move it along but we're talking internally about focusing more on the Travis build rather than AppVeyor for user-contributed PRs such as this. That's part of the rationale for PYTHON-1362. I certainly don't object to upgrading AppVeyor, however, so there's no real problem there.

Full review to come!

absurdfarce

Still working on a full review but I wanted to get info about these changes out as quickly as possible.

The canonical test setup for the Python driver is our Jenkins server which runs a full unit and integration test suite against a set of Apache Cassandra and DSE versions (neither Travis nor AppVeyor run the full integration test suite). I setup a build of these changes on that server and observed several test failures due to a few nits that come along with the removal of six. The comments in this review fixed all of the new issues introduced by this PR, so my expectation is that incorporating the changes referenced in this review will go a long way towards getting this in.

@Lorak-mmk and/or @fruch : if you guys are able to include these changes in this PR (and possibly in your downstream fork) that would be great. I'm also fine including them in a follow-on commit after we get the work in this PR merged. Just let me know what you want to do.

absurdfarce · 2023-10-10T19:10:48Z

cassandra/datastax/graph/graphson.py

@@ -141,7 +135,7 @@ def serialize(cls, value, writer=None):

    @classmethod
    def get_specialized_serializer(cls, value):
-        if type(value) in six.integer_types and (value > MAX_INT32 or value < MIN_INT32):
+        if type(value) in int and (value > MAX_INT32 or value < MIN_INT32):


This should be the following instead:

if type(value) == int and (value > MAX_INT32 or value < MIN_INT32):

six.integer_types is just "int" in the Python3 case (it is a seq for Python2) so I'm not sure how this wasn't a problem before.

absurdfarce · 2023-10-10T19:13:25Z

tests/integration/advanced/graph/fluent/test_graph.py

            typ, value, deserializer = data
            vertex_label = VertexLabel([typ])
-            property_name = next(six.iterkeys(vertex_label.non_pk_properties))
+            property_name = next(vertex_label.non_pk_properties.keys())


keys() on a dict returns a dictionary view object rather than a sequence so this needs to be explicitly converted to an iterator. So this should instead be:

property_name = next(iter(vertex_label.non_pk_properties.keys()))

absurdfarce · 2023-10-10T19:14:27Z

tests/integration/advanced/graph/fluent/__init__.py

            typ, value, deserializer = data
            vertex_label = VertexLabel([typ])
-            property_name = next(six.iterkeys(vertex_label.non_pk_properties))
+            property_name = next(vertex_label.non_pk_properties.keys())


Another case where we need an iter:

property_name = next(iter(vertex_label.non_pk_properties.keys()))

absurdfarce · 2023-10-10T19:15:06Z

tests/integration/advanced/graph/fluent/__init__.py

            vertex_label = VertexLabel([typ])
-            property_name = next(six.iterkeys(vertex_label.non_pk_properties))
+            property_name = next(vertex_label.non_pk_properties.keys())


Another case where we need an iter:

property_name = next(iter(vertex_label.non_pk_properties.keys()))

absurdfarce · 2023-10-10T19:16:03Z

tests/integration/advanced/graph/test_graph_datatype.py

            typ, value, deserializer = data
            vertex_label = VertexLabel([typ])
-            property_name = next(six.iterkeys(vertex_label.non_pk_properties))
+            property_name = next(vertex_label.non_pk_properties.keys())


Another case where we need an iter:

property_name = next(iter(vertex_label.non_pk_properties.keys()))

absurdfarce · 2023-10-10T19:17:43Z

tests/integration/advanced/graph/test_graph_datatype.py

            vertex_label = VertexLabel([typ])
-            property_name = next(six.iterkeys(vertex_label.non_pk_properties))
+            property_name = next(vertex_label.non_pk_properties.keys())


Another case where we need an iter:

property_name = next(iter(vertex_label.non_pk_properties.keys()))

absurdfarce · 2023-10-10T19:19:06Z

tests/integration/advanced/graph/test_graph_query.py

@@ -588,7 +587,7 @@ def _test_basic_query_with_type_wrapper(self, schema, graphson):
        vl = VertexLabel(['tupleOf(Int, Bigint)'])
        schema.create_vertex_label(self.session, vl, execution_profile=ep)

-        prop_name = next(six.iterkeys(vl.non_pk_properties))
+        prop_name = next(vl.non_pk_properties.keys())


Another case where we need an iter:

prop_name = next(iter(vl.non_pk_properties.keys()))

six.iterkeys() returns an iterator, but Python's dict.keys() does not, so to pass it to iter() it needs to be first passed trough iter().

Lorak-mmk · 2023-10-16T19:54:30Z

I fixed the issues you pointed out, sorry it took so long. We didn't notice them before because we don't run DSE integration tests

absurdfarce · 2023-10-27T19:21:34Z

No worries @Lorak-mmk , thanks for the quick turnaround! The rest of this looks pretty good so I think we're largely ready to merge this guy (and finally be completely rid of six!)

fruch · 2023-11-02T15:38:53Z

No worries @Lorak-mmk , thanks for the quick turnaround! The rest of this looks pretty good so I think we're largely ready to merge this guy (and finally be completely rid of six!)

@absurdfarce

You might want take a look at also on:

scylladb#260

It's paving most of the way for python 3.12 support

absurdfarce · 2023-11-06T19:52:02Z

Thanks for the pointer @fruch! I'll add a comment on the ongoing work on Python 3.12 support on our side there.

'six' was removed in Apache Cassandra 4.1 (CASSANDRA-17417) and from the Python Driver in 3.29 datastax/python-driver#1172, but we forgot to update `setup.py` when we pulled that in.

…sync_with_upstream_3.29.1 version 3.29.0 * tag '3.29.0' of https://github.com/datastax/python-driver: Documentation (and other) updates for 3.29.0 (datastax#1194) PYTHON-1331 ssl.match_hostname() is deprecated in 3.7 (datastax#1191) PYTHON-1371 Add explicit exception type for serialization failures (datastax#1193) Remove outdated Python pre-3.7 references (datastax#1186) PYTHON-1368 Avoid installing DSE deps + executing DSE tests for Python 3.12 (datastax#1188) PYTHON-1366 Handle removal of asyncore in Python 3.12 (datastax#1187) Removed backup(.bak) files (datastax#1185) docs: Fix typo in add_callbacks (datastax#1177) Remove some remnants of Python2 (datastax#1172) PYTHON-1313 Fix asyncio removals in Python 3.10 (datastax#1179) PYTHON-1364 Fix ssl.wrap_socket errors (from eventlet) for Python 3.12 (datastax#1181) Add Jenkins support for Python 3.12.0 (datastax#1180) Update redirects in docs.yaml (datastax#1178) Jenkins using new python versions in the matrix (datastax#1174) Update docs.yaml to point to most recent 3.28.0 docs changes CONN-38 Notes for 3.28.0 on PYTHON-1350 (datastax#1167) Include docs for 3.28.0 Fixed non-valid rst in README

Lorak-mmk added 2 commits July 19, 2023 19:04

Drop 'six' from dependencies

d9322dc

As we no longer support Python 2, there is no reason to keep this dependency. This commit removes all usages of six and removes it from dependencies.

Lorak-mmk mentioned this pull request Jul 19, 2023

Remove some remnants of Python2 scylladb/python-driver#243

Merged

absurdfarce self-requested a review July 20, 2023 06:10

compatibility with python 3.7 only

fc372e6

DROP THIS: changed gitignore

d7bd6d1

absurdfarce mentioned this pull request Oct 9, 2023

Fix to remove python 2 & python 3 switches using if statements #1168

Closed

absurdfarce requested changes Oct 10, 2023

View reviewed changes

Fix problems introduced while removing six

6159430

six.iterkeys() returns an iterator, but Python's dict.keys() does not, so to pass it to iter() it needs to be first passed trough iter().

absurdfarce merged commit d0e639b into datastax:master Oct 27, 2023

bschoening mentioned this pull request Apr 12, 2024

Remove unused six from setup.py jeffwidman/cqlsh#37

Merged

bschoening referenced this pull request in Homebrew/homebrew-core May 6, 2024

cassandra: migrate to [email protected]

7bad50e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove some remnants of Python2 #1172

Remove some remnants of Python2 #1172

Lorak-mmk commented Jul 19, 2023

fruch commented Jul 19, 2023

Lorak-mmk commented Jul 19, 2023 •

edited

Loading

absurdfarce commented Jul 20, 2023

Lorak-mmk commented Jul 20, 2023

bschoening commented Aug 3, 2023 •

edited

Loading

Lorak-mmk commented Aug 3, 2023

vgali7 commented Aug 3, 2023

fruch commented Aug 29, 2023

bschoening commented Aug 30, 2023

absurdfarce commented Aug 30, 2023

bschoening commented Aug 31, 2023 •

edited

Loading

absurdfarce commented Oct 9, 2023 •

edited by jira bot

Loading

absurdfarce left a comment

absurdfarce Oct 10, 2023

absurdfarce Oct 10, 2023

absurdfarce Oct 10, 2023

absurdfarce Oct 10, 2023

absurdfarce Oct 10, 2023

absurdfarce Oct 10, 2023

absurdfarce Oct 10, 2023

Lorak-mmk commented Oct 16, 2023

absurdfarce commented Oct 27, 2023

fruch commented Nov 2, 2023

absurdfarce commented Nov 6, 2023

Remove some remnants of Python2 #1172

Remove some remnants of Python2 #1172

Conversation

Lorak-mmk commented Jul 19, 2023

fruch commented Jul 19, 2023

Lorak-mmk commented Jul 19, 2023 • edited Loading

absurdfarce commented Jul 20, 2023

Lorak-mmk commented Jul 20, 2023

bschoening commented Aug 3, 2023 • edited Loading

Lorak-mmk commented Aug 3, 2023

vgali7 commented Aug 3, 2023

fruch commented Aug 29, 2023

bschoening commented Aug 30, 2023

absurdfarce commented Aug 30, 2023

bschoening commented Aug 31, 2023 • edited Loading

absurdfarce commented Oct 9, 2023 • edited by jira bot Loading

absurdfarce left a comment

Choose a reason for hiding this comment

absurdfarce Oct 10, 2023

Choose a reason for hiding this comment

absurdfarce Oct 10, 2023

Choose a reason for hiding this comment

absurdfarce Oct 10, 2023

Choose a reason for hiding this comment

absurdfarce Oct 10, 2023

Choose a reason for hiding this comment

absurdfarce Oct 10, 2023

Choose a reason for hiding this comment

absurdfarce Oct 10, 2023

Choose a reason for hiding this comment

absurdfarce Oct 10, 2023

Choose a reason for hiding this comment

Lorak-mmk commented Oct 16, 2023

absurdfarce commented Oct 27, 2023

fruch commented Nov 2, 2023

absurdfarce commented Nov 6, 2023

Lorak-mmk commented Jul 19, 2023 •

edited

Loading

bschoening commented Aug 3, 2023 •

edited

Loading

bschoening commented Aug 31, 2023 •

edited

Loading

absurdfarce commented Oct 9, 2023 •

edited by jira bot

Loading