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

Compatibility For Python3 #122

Closed
wants to merge 1 commit into from
Closed

Compatibility For Python3 #122

wants to merge 1 commit into from

Conversation

dcolish
Copy link
Contributor

@dcolish dcolish commented Mar 24, 2011

I've completed the changes required to have redis-py work with python 2 and python 3. The only issue outstanding is that hiredis-py will not work with python 3. I have begun that port and made some good progress, but there are still a few unfinished items. I'm not sure its a blocker for us.

@andymccurdy
Copy link
Contributor

Awesome, thanks Dan. Going apply this soon. I've not done much with Python 3 yet, so I'm using your changes to familiarize myself with Python 3.

@dcolish
Copy link
Contributor Author

dcolish commented Apr 15, 2011

Thanks! I think its a little clearer what is happening in python3 vs python2 when done this way versus 2to3. The differences really boil down to just a few type changes.

@andymccurdy
Copy link
Contributor

I'll be merging this stuff in once the connection_refactor branch goes in. Thanks again for the work on this Dan.

@dcolish
Copy link
Contributor Author

dcolish commented May 25, 2011

Sweet! I'll check out the connection_refactor too since it probably effects code I have

@andymccurdy
Copy link
Contributor

connection_refactor has been merged to master and rolled into redis-py 2.4.0. I'd like to get to adding Python 3 support next. I'll start looking at your patch over the next couple days.

@dcolish
Copy link
Contributor Author

dcolish commented Jun 1, 2011

sounds good. there shouldnt be any issues, but there might be some changes that are needed in the connection_refactor branch.

@dcolish
Copy link
Contributor Author

dcolish commented Jun 23, 2011

I attempted a merge today and got multiple conflicts. It will require a little work to merge this in with the connection refactor.

@dag
Copy link

dag commented Aug 25, 2011

Why not use_2to3 with distribute, and test it with tox? Keeps the code cleaner and lets you get away with writing mostly like you're used to, and still use things like iteritems in 2.x.

@dcolish
Copy link
Contributor Author

dcolish commented Aug 25, 2011

Well, since this patch will certainly not apply cleanly to the redis-py/master, something has to be done to bring in python3 compatibility. However, I've never been happy with 2_to_3's results. This patch was a very small amount of code to manage the compatibility without having to compile code at deployment time; a small amount of code which I have not had the time to maintain. We did look at the 2_to_3 option initially and we would likely need fixers for 2_to_3 to apply correctly which means maintaining code anyway. I'm not sure where that comes out as cleaner. Anyway, if you present a patch, I'm open to new ideas.

I do like the idea of using Tox for testing; any help getting that setup is greatly appreciated since I have little experience with it and little time.

@atomAltera
Copy link

I can`t write bytes to database

import redis
r = redis.Redis()
f = open(r"E:\IMG.JPG", 'br')
data = f.read()
r.set("data", data)

Writes:
Traceback (most recent call last):
File "E:/testing/Testing (Python32)/main.py", line 9, in
r.set("data", data)
File "F:\Program Files\Python32\lib\site-packages\redis\client.py", line 576, in set
return self.execute_command('SET', name, value)
File "F:\Program Files\Python32\lib\site-packages\redis\client.py", line 264, in execute_command
enc_value.decode())]
UnicodeDecodeError: 'utf8' codec can't decode byte 0xff in position 0: invalid start byte

@andymccurdy
Copy link
Contributor

@atomAltera What version of redis-py are you using? And are you running on Python 3?

@atomAltera
Copy link

@andymccurdy
redis-py 2.2.3 ( dcolish@63e3414 )
Python 3.2
Redis 2.4.5

Why redis-py trys to decode binary data to utf-8 string?

@lclarkmichalek
Copy link

I've implemented python3 compatibility in my fork: https://github.com/bluepeppers/redis-py

All tests but one pass on python 3 (and I'm not sure what's up with the failing one), and all pass on python 2.

@atomAltera
Copy link

And what about this one dcolish@63e3414 ?

@lclarkmichalek
Copy link

That's someone else's repo. I don't know about it, but it seems to be the one you had problems with bytes on. My implementation doesn't call decode on user data, so all commands return bytes, not unicode, and you should be able to store binary data fine.

@dahlia
Copy link

dahlia commented May 3, 2012

Test suite seems to have to be run using tox. It helps tests to be run on multiple environments (various Python versions e.g. Python 2.6, 2.7, 3.2 and various Python implementations e.g. PyPy, Jython).

@atomAltera
Copy link

I Can't install it on python3.2 (Windows XP)

Traceback (most recent call last):
File "setup.py", line 3, in
from redis import version
File "C:\Documents and Settings\Admin\Рабочий стол\bluepeppers-redis-py-281c8ef\redis__init__.py", line 1, in
from redis.client import Redis, StrictRedis
File "C:\Documents and Settings\Admin\Рабочий стол\bluepeppers-redis-py-281c8ef\redis\client.py", line 14, in
from redis.connection import ConnectionPool, UnixDomainSocketConnection
File "C:\Documents and Settings\Admin\Рабочий стол\bluepeppers-redis-py-281c8ef\redis\connection.py", line 2, in
import socket
File "F:\Program Files\Python32\lib\socket.py", line 46, in
import _socket
ImportError: Module use of python26.dll conflicts with this version of Python.

On Debian with python3.1 works

@lclarkmichalek
Copy link

@atom, I think that's a problem with your python 3 install, try doing 'import socket' in your python 3 repl.

@dahlia, I'll try using tox, I was just using nose with cpython 2.7 and stackless 3.2. Have you/redis-py concidered using Travis-ci to automatically run the tests over a fairly large range of cpython versions?

@andymccurdy
Copy link
Contributor

Travis-ci would definitely help things here. One of the reasons I haven't merged this previously is I don't have a Python 3 environment that I actively use, so committing to supporting it has been a challenge. Punting that over to Travis-ci would certainly lessen the burden.

@atomAltera
Copy link

@bluepeppers finely, it`s work. Thanks

@lclarkmichalek
Copy link

@andymccurdy, Travis-ci supports redis out of the box, so setting it up shouldn't be hard. Also, I've only tested my port on python 2.7, what's the lowest version of python that redis-py supports?

@andymccurdy
Copy link
Contributor

@bluepeppers Currently 2.5. It needs the with statement.

@dcolish
Copy link
Contributor Author

dcolish commented May 3, 2012

I've abandoned work on my branch. It was working at the time I ported it but has been left to diverge greatly from the current redis-py. I cannot make any guarantees that it would still work.

@lclarkmichalek
Copy link

Ok, my fork now works from 2.5 to 3.2, and I've set up travis ci on my repo: http://travis-ci.org/#!/bluepeppers/redis-py.

It should be fine to merge now if you want to. Things to note going forward are that when comparing between responses from redis and string literals, the string literal will need to have encode called on it, and that to get an exception after catching it, use sys.exec_info()[1].

@atomAltera
Copy link

@bluepeppers how you convert input values to bytes? str(value).encode()?

@lclarkmichalek
Copy link

Input values are automatically encoded here: https://github.com/bluepeppers/redis-py/blob/master/redis/connection.py#L287

Basically, test if they're bytes already, if not then call unicode on them (if they are already unicode this will have no effect), then encode them with the given encoding.

@atomAltera
Copy link

@bluepeppers nice.
Are there another differences from dcolish@63e3414 @dcolish 's redis-py ?

@lclarkmichalek
Copy link

There are no differences in the API other than the bytes/unicode issue that I can see.

@xpahos
Copy link

xpahos commented May 25, 2012

Maybe it will be better to create separate package named e.g. redis-py3 not compatible with python 2.x?

@andymccurdy
Copy link
Contributor

Python 3.x support has landed in redis-py master. It will be officially released in 2.6.1. Would love to hear feedback from anyone who would like to help test against master.

@andymccurdy
Copy link
Contributor

Python 3.x support has been officially released as of 2.6.2. Closing this issue.

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.

None yet

8 participants