-
Notifications
You must be signed in to change notification settings - Fork 115
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 Compatibility #57
Conversation
There is a warning in the docs about
which does indeed appear to happen with >>> try:
... raise Exception()
... except:
... _, a, _ = sys.exc_info()
... print(_)
...
<traceback object at 0x1073eedd0> I think compatibility with 2.4 should be dropped by using the |
Ah, thanks for that. I'll push -f a fixed version in a bit. And no, I won't drop 2.4 support in patches I submit, as I need it 😄 |
b10209d
to
5af6650
Compare
👍 |
EDIT: Emoticons, how do they work? |
Thanks for your work on this, @seveas. The continued support for Python 2.4 is much appreciated, I know from a few beanstalkc users who rely on Python 2.4; so also thanks for that on their behalf. I cherry-picked your exception handling fix, so that we have at least this part settled. As for the other parts introducing Py3 compatibility, there's this design issue I mentioned in #13 with 8-bit transparency in combination with cross-Python 2/3 support. Your "PY3" version switch nicely manages to keep 8-bit transparency working properly on Py2, but doesn't provide similar functionality on Py3. How to do that nicely on Py3 is mainly a question of design. Since this issue seems to be little known, I'll write it up in a separate ticket. I'll leave your pull request pending, for the time being, as it's quite conceivable to implement whatever 8-bit transparency design we come of for Py3 on top of it. |
@earl When will you merge this pr? |
Can this be rebased/merged? Happy to help in any form |
Under python3, we are responsible for decoding and encoding. The implementation does not change the behaviour for python 2, and by default uses python2-esque behaviour (using sys.getdefaultencoding()) under python 3, but lets the user override the encoding or even set it to None to get bytes objects back. It also allows the user to pass bytes objects to put, striking the best balance between backwards compatibility for binary data in tubes (which can never be achieved 100%) and working nicely with text data. Tested with python 2.4, 2.7 and 3.4
5af6650
to
b626230
Compare
b626230
to
34bbada
Compare
Rebased and updated to include the suggestion I made on #60. |
What's going on here – can this be merged? Let me know if there's anything I can do. I'm tempted to fork this repo for now since this is holding us back from migrating our stack to Python 3 |
@erikbern I think you can fork & rebase this patch in your sub stream, install it via editable mode. |
Yes of course – would just be nice not to have to rely on our own custom verision |
I've tested this with Python 3.5 and it works well, thanks. Can we merge this? Currently my requirements.txt uses:
|
@earl Any update on when this might be merged? |
PY3 = sys.version_info[0] > 2 | ||
if PY3: | ||
b = lambda x: isinstance(x, bytes) and x or bytes(x, 'us-ascii') | ||
s = lambda x: x.decode('us-ascii') |
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.
Any reason not to do the following?
b = lambda x: x if isinstance(x, bytes) else x.encode()
s = lambda x: x.decode()
This would change the encoding from us-ascii
to utf-8
, but I am not sure why you chose us-ascii
in the first place.
For what it is worth, an other project facing a similar issue, redis-py, chose to go accept either By the way, it may be worth it to look at how they implement Python2/3 compatibility. I think it is fine to do the encoding/decoding automagically, as long as the user is able to bypass it if she chooses to do so (here by setting I've added some inline comments, but good job, take my 👍 |
Hello @seveas, could you please bump the version number in your branch? Also if you have some time to review my last comment, it would be appreciated :-) |
👍 |
Would be nice if this can be merged in |
Is there any timeline on when me might be able to expect python3 support to be merged? |
I'm happy with this being merged... while I'm not actively using it, I have used this specific branch & revision on both 2.7 and 3.4 and 3.5 and it seems to work fine. |
Paging @seveas 👋: could you please take a few minutes to review my comments? 👼 I know this issue is starting to be old and all, but it would be appreciated 💛 |
Any update here? Python 3 support is really needed. |
@rwarren I solved it by switching to: https://pypi.python.org/pypi/beanstalkc3. |
@svisser Perfect... thanks for the PyPi link! |
These three patches make beanstalkc compatible with python 2.4-3.4. Possibly
even 2.3, but I don't have that installed anywhere to test.