aio-libs-abandoned / aioredis-py

asyncio (PEP 3156) Redis support
https://aioredis.readthedocs.io/
MIT License
2.3k stars 336 forks source link

Per-request decoding basis (defers from redis-py) #1117

Open samuelcolvin opened 3 years ago

samuelcolvin commented 3 years ago

Hi, great to see aioredis is getting some love.

Running the following:

import asyncio
import aioredis

async def main():
    redis = aioredis.from_url('redis://localhost', encoding='utf-8')
    await redis.set('my-key', 'value')
    value = await redis.get('my-key')
    print(value)

if __name__ == '__main__':
    asyncio.run(main())

is printing b'value' - it seems like the encoding argument to from_url is being ignored (at least I couldn't find documentation on the encoding argument, so maybe it doesn't do what I expected).

More generally, in migration arq to use aioredis 2 (https://github.com/samuelcolvin/arq/pull/259) it would be extremely useful if we could decode the result of commands on a per-command basis, rather than on a connection basis. Is this something you'd consider supporting?

samuelcolvin commented 3 years ago

i see decode_responses now, sorry.

Still, for arq we really need decode_response on a per request basis, is that something you'd consider?

Andrew-Chen-Wang commented 3 years ago

So aioredis 2 is basically a port of redis-py, so for the most part, if there's a problem, you could head to the redis-py docs or Google with a redis-py keyword term and it should be mostly the same (besides some API usage).

@abrookins It's up to you pertaining to the per request basis; I've got school and I think @seandstewart planned on being busy around this time of the year.

Just my opinion though, I don't see a problem with a per-request basis so long as there is minimal code changes (i.e. we shouldn't have to go through each command and add in 2 lines of code amounting to 100s of lines of code of maintenance in case something changes that all 3 of us don't really have time to constantly update).

Yolley commented 3 years ago

Actually it seems that redis-py maintainer at one point thought of adding response_context that would allow to change encoding temporarily inside a context manager https://github.com/redis/hiredis-py/pull/96