-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Python 3 Support #227
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
Python 3 Support #227
Conversation
…into feature/py3
"All is well". Niiiiiice. What do you say to some Python 3 support? |
Python 2 returns a value in the range [-2**31, 2**31-1]. | ||
Python 3 returns a value in the range [0, 2**32-1]. | ||
|
||
We want a consistent behavior so let's use python2's. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems wrong to me. see https://docs.python.org/3.1/library/binascii.html#binascii.crc32
CRC is only important in binary pack form b/c that is what is sent to the broker. does struct.pack('>i', crc) behave differently on py2 v. py3 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears so:
kafka-python $ python3
Python 3.4.1 (default, Sep 3 2014, 10:12:05)
>>> import struct, binascii
>>> struct.pack('<i', binascii.crc32(b'abcd'))
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
struct.error: 'i' format requires -2147483648 <= number <= 2147483647
kafka-python $ python
Python 2.7.6 (default, Sep 3 2014, 10:15:09)
>>> import struct, binascii
>>> struct.pack('<i', binascii.crc32(b'abcd'))
'\x11\xcd\x82\xed'
+1 but needs rebase to merge automatically |
nice! big +1 for this |
Updates PR #224, with fixed tests.