search

leporo / tornado-redis

Asynchronous Redis client that works within Tornado IO loop.
667 stars 163 forks source link

Removed unicode encoding for strings that might not be unicode #50

Open njo opened 10 years ago

njo commented 10 years ago

Reverted where the code was attempting to unicode encode a value pulled from redis. The to_unicode would cause an exception to be thrown if there was a multibulk reply (eg. HMGET myhash field1 field2) and the values stored in redis were not unicode data - ie. gzip data.

leporo commented 10 years ago

Please check out the unit-test output: https://travis-ci.org/leporo/tornado-redis/builds/14397546 I can't merge your change at the moment.

BTW, tornado-redis' performance with MGET/MSET commands is really poor. May I suggest you to try using redis-py (synchronous client) in your application for MGET/MSET commands? Please benchmark your code to be sure it performs better. Feel free to ask me if you need any assistance.

See the issue #36 for details on poor MGET/MSET performance.

I think we may get far better performance by using the hiredis parser for reply processing. There are just too many callbacks in multi-bulk reply handling.

gmr commented 10 years ago

The unicode bit is biting me with binary data as well. Any chance we can see this addressed? @leporo is the main issue the tests needing updates?

leporo commented 10 years ago

The main issue with the proposed fix is that it may ruin existing applications.

I'll review client's code when I have spare time.

rudolphfroger commented 9 years ago

Please fix this, putting binary data into Redis is quite common I believe (like struct packed integers, hash digests etc). Contact me if there's anything I can do to help with this.

zjjott commented 8 years ago

+1