Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Drop python support for 2.7, 3.4 and 3.5 #321

Merged
merged 4 commits into from
Jul 20, 2021
Merged

Drop python support for 2.7, 3.4 and 3.5 #321

merged 4 commits into from
Jul 20, 2021

Conversation

jogo
Copy link
Contributor

@jogo jogo commented May 27, 2021

End of Life information:

Remove six dependency and run pyupgrade to start using native python 3 syntax

@jogo jogo requested review from cgordon and jparise as code owners May 27, 2021 16:52
@jogo jogo force-pushed the EOL branch 2 times, most recently from a89a551 to 243e043 Compare May 27, 2021 17:04
@jparise jparise added this to the 4.0 milestone May 27, 2021
4383
4383 suggested changes Jun 28, 2021
View reviewed changes
Copy link
Contributor

@4383 4383 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,

Please see my inline comments, I think that we need few minor adjustements.
Else everything seems ok.

pymemcache/test/test_serde.py Outdated Show resolved Hide resolved
pymemcache/serde.py Show resolved Hide resolved
pymemcache/serde.py Outdated Show resolved Hide resolved
@4383 4383 mentioned this pull request Jun 28, 2021
Merged
4383
4383 reviewed Jul 7, 2021
View reviewed changes
Copy link
Contributor

@4383 4383 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The usage of f-string LGTM. This feature has been introduced with python 3.6 and we drop the previous versions (3.4, 3.5).

pymemcache/client/base.py Outdated Show resolved Hide resolved
@nichochar
Copy link
Collaborator

Very exciting that we're finally dropping support.
Looked at the changes, and LGTM, but haven't been as close to the repo in a while so I'll let someone else gatekeep.

Also, if things are missing, I wouldn't find those, but changes made all LGTM.

@jogo jogo force-pushed the EOL branch 2 times, most recently from 7e7742a to d03a6d6 Compare July 13, 2021 23:27
jparise
jparise approved these changes Jul 14, 2021
View reviewed changes
Copy link
Collaborator

@jparise jparise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me overall. I'd still like to hold off on merging it for a few days since we just released v3.5.0. If nothing significant pops up from that release, we can proceed with a Python 3-only v4 and create a v3.x branch if necessary.

pymemcache/client/rendezvous.py Outdated Show resolved Hide resolved
pymemcache/test/test_benchmark.py Outdated
@@ -103,7 +102,7 @@ def test_bench_set_multi(request, client, pairs, count):

@pytest.mark.benchmark()
def test_bench_delete(request, client, pairs, count):
benchmark(count, client.delete, six.next(six.iterkeys(pairs)))
benchmark(count, client.delete, next(pairs.keys()))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be just next(pairs)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it could be https://six.readthedocs.io/#six.iterkeys

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could drop the .keys() here.

@4383
Copy link
Contributor

4383 commented Jul 15, 2021

Creating a branch 3.X seems a good idea to backport and fix issues.
It could even be created later by referring to the right hash if something happen with previous features in the case it will require backward compatibility.

jparise
jparise reviewed Jul 20, 2021
View reviewed changes
pymemcache/client/base.py Show resolved Hide resolved
pymemcache/test/test_benchmark.py Outdated
@@ -103,7 +102,7 @@ def test_bench_set_multi(request, client, pairs, count):

@pytest.mark.benchmark()
def test_bench_delete(request, client, pairs, count):
benchmark(count, client.delete, six.next(six.iterkeys(pairs)))
benchmark(count, client.delete, next(pairs.keys()))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could drop the .keys() here.

jogo added 4 commits July 20, 2021 11:46
@jogo
Drop python support 2.7, 3.4 and 3.5
fe2114f 
Add Python 3 only classifier
Stop running Python 2 tests

End of Life information:
* 3.5 https://www.python.org/downloads/release/python-350/
* 3.4 https://www.python.org/downloads/release/python-340/
* 2.7 https://www.python.org/doc/sunset-python-2/
@jogo
Remove six dependency and run pyupgrade
0bf1baa 
Now that we don't require Python 2 support no need for six.

Code upgraded with pyupgrade and manual fixes to remove remaining six
usage.
@jogo
Remove PY2 specific imports and tests
00f3dd5 
* future was needed for some python 2 specific tests
* use mock from unittest.mock instead of the 3rd party mock library
@jogo
Fixes for rebase and further cleanups
09e4c05
@jparise jparise merged commit b6d83f8 into pinterest:master Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@4383 4383 4383 requested changes

@jparise jparise jparise approved these changes

@cgordon cgordon Awaiting requested review from cgordon

Assignees
No one assigned
Labels
python3
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.
4 participants
@jogo @nichochar @4383 @jparise