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.

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

Fix for python3 issues #36

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Fix for python3 issues #36

wants to merge 10 commits into from

Conversation

thoas
Copy link

@thoas thoas commented Jul 29, 2015

  • Add a compat layer for missing methods or moved methods (based on six)
  • Fix bytes issues when using crc32
  • Some flake8 issues

Available if you want more information.

@mattrobenolt
Copy link
Collaborator

@thoas looks like all of the python 3 tests fail because our dependencies aren't python 3 compatible. :(

@thoas
Copy link
Author

thoas commented Jul 29, 2015

@mattrobenolt yep, it seems pycassa is not compatible at all with python3 : even the current master doesn't pass at all.

Do you have a recommendation for this case? I think we need to disable pycassa tests for py3.

@mattrobenolt
Copy link
Collaborator

I'm not sure off the top of my head. I think that's reasonable though if that's the only incompatibility.

tbh though, I'd like to rip out pycassa support since that's silly in nydus. :) Not sure why it was a thing in the first place.

@mattrobenolt
Copy link
Collaborator

Also, pycassa is deprecated in favor of https://github.com/datastax/python-driver, so I'd be open to ripping that out if we get full python 3 compat without it.

thoas added 3 commits July 30, 2015 10:01
* Rename assertEquals to assertEqual
* Rename assertNotEquals to assertNotEqual
* Add httplib and next in compat
* Fix map use cases which does not return a list anymore in py3
@thoas
Copy link
Author

thoas commented Jul 30, 2015

@mattrobenolt I have disabled pycassa tests in Python 3 context.
I think, it would be better to remove the support in another PR.

I need your brain on a nasty choice in Python 3, mine is melting.

======================================================================
ERROR: test_nonzero (tests.nydus.db.connections.tests.EventualCommandTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/thoas/Sites/Python/ulule/nydus/tests/nydus/db/connections/tests.py", line 354, in test_nonzero
    self.assertEqual(int(ec or 0), 0)
  File "/Users/thoas/Sites/Python/ulule/nydus/nydus/db/promise.py", line 140, in __len__
    return len(self.__wrapped)
TypeError: object of type 'NoneType' has no len()

I lost the self when resolving a promise to None which is ok since it's the exact behavior on Python 2, but ec or 0 in Python 3 is stricter and raises an exception instead of keeping the 0 default value.

What do you suggest for these case scenario?

@thoas
Copy link
Author

thoas commented Aug 4, 2015

@mattrobenolt 🎉 🎉 tests are passing on py2/py3

Give me your feedback when you will have the time :)

@thoas
Copy link
Author

thoas commented Aug 9, 2015

@mattrobenolt any time on your side to review this PR?

Thanks :)

@mattrobenolt
Copy link
Collaborator

This mostly seems ok to me.

/cc @dcramer opinions?

@dcramer
Copy link
Collaborator

dcramer commented Aug 10, 2015

idc you guys are in charge :)

@mattrobenolt
Copy link
Collaborator

:rage4:

@mattrobenolt
Copy link
Collaborator

I'm going to review this and compare with my progress that I made on my py3 branch from a while ago that I didn't finish.

master...py3 for reference.

@thoas
Copy link
Author

thoas commented Aug 10, 2015

We are in charge 😱 😱

@thoas
Copy link
Author

thoas commented Sep 2, 2015

@mattrobenolt any feedback on this?

Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants