migrate existing context manager wrapper v0.3 to v1.0

[marcstreeter]

I've been actively using aioredis v0.3 up until I attempted to move to python 3.7 which triggered the need to do an update to v1. I've put aioredis inside a wrapper class that retains this interface:

     from .util import RedisWrapper
     
     async with RedisWrapper() as r:
         keys = await r.get("*")  # cringeworthy I know, just an example

I have a feeling that I may have been using it wrong all along as my set up totally crumbled with the move.

Here's my current set up under 0.3

    import aioredis
    from aioredis import RedisPool
    import config
    
    
    class RedisWrapper:
        __slots__ = ("_instance", )
        _pool = None
    
        def __init__(self):
            self._instance = None
        
        async def __aenter__(self):
            pool = await self.get_pool()
            self._instance = await pool.acquire()
            return self._instance
        
        async def __aexit__(self, exc_type, exc_val, exc_tb):
            pool = await self.get_pool()
            pool.release(self._instance)
            self._instance = None
        
        async def get_pool(self) -> RedisPool:
            if not self._pool:
                self._pool = await aioredis.create_pool(
                    (config.host, config.port),
                    minsize=5,
                    maxsize=10
                )
            
            return self._pool

I've been reading through the migration description and it looks like interaction has changed such that now I'm trying to make the move to the aioredis.create_redis_pool (as that seems like the closest fit). After many failed attempts, however, I don't know how to (or if I even have to) release the connection during the clean up in __aexit__. I've been looking around to see what others do and I've noticed the following usage in aioredlock (line 118-128):

    async def connect(self):
        # I cut out a lot of code to focus on this
        if self._pool is None:
            async with self._lock:
                if self._pool is None:
                    self.log.debug('Connecting %s', repr(self))
                    self._pool = await self._create_redis_pool(
                        address, **redis_kwargs,
                        minsize=1, maxsize=100)
                    with await self._pool as redis:
                        await self._register_scripts(redis)

        return await self._pool

and then interacted with here(line 142-147):

    with await self.connect() as redis:
        await redis.evalsha(
            self.set_lock_script_sha1,
            keys=[resource],
            args=[lock_identifier, lock_timeout_ms]
        )

which leads me to believe I could just do this in v1:

    import aioredis
    import config
    
    class RedisWrapper:
        __slots__ = ("_instance", )
        _pool = None
    
        def __init__(self):
            self._instance = None
    
        async def __aenter__(self):
            self._instance = await self.get_pool()
            return await self._instance
    
        async def __aexit__(self, exc_type, exc_val, exc_tb):
            self._instance = None
    
        async def get_pool(self):
            if not self._pool:
                self._pool = await aioredis.create_redis_pool(
                    (config.host, config.port),
                    minsize=5,
                    maxsize=10
                )
    
            return self._pool

This set up seems to work, but I'm just copying code, and would like a confirmation that I'm not abusing the library.

[marcstreeter]

k- so I'm already see problems with above (the pool is not releasing it's connections). This is what I had to do (and it seems to work, but again I'm not sure if I'm doing bad things to this library)

    import aioredis
    import config
    
    class RedisWrapper:
        __slots__ = ("_instance", )
        _pool = None
    
        def __init__(self):
            self._instance = None
    
        async def __aenter__(self):
            self._instance = await self.get_pool()
            return self._instance # still don't know what aioredlock was achieving with the await
    
        async def __aexit__(self, exc_type, exc_val, exc_tb):
            await self._instance.quit()  # releases "the connection" which I'm assuming means the connection I had(and not some other connection), must be awaited or doesn't work reliably -  again I'm guessing
            self._instance = None
    
        async def get_pool(self):
            if not self._pool:
                self._pool = await aioredis.create_redis_pool(
                    (config.host, config.port),
                    minsize=5,
                    maxsize=10
                )
    
            return self._pool

[akhilman]

First of all seems you need create_redis_pool(), not create_pool()
create_redis_pool() provides commands as methods, like pool.get(key)
create_pool() yields raw stream with exec() method - pool.exec('get', key)
https://aioredis.readthedocs.io/en/v1.1.0/start.html#python-3-5-async-with-async-for-support

Then you really do not need acquire connection from pool for non-blocking commands.
You may call methods directly from pool - pool.get(key).
https://aioredis.readthedocs.io/en/v1.1.0/migration.html#blocking-operations-and-connection-sharing

[marcstreeter]

Just in case anyone here is only following the aioredis thread:

From a conversation with @akhilman and some more investigation into aioredis the current implementation has shifted back to using create_pool (I had original shifted to using create_redis_pool) as shown in my above comment. Here's the current implementation:

     import aioredis
     from aioredis import ConnectionsPool, Redis
     import config
    
    class RedisWrapper:
        __slots__ = ("_instance", )
        _pool = None
    
        def __init__(self):
            self._instance = None
    
        async def __aenter__(self) -> Redis:
            pool = await self.get_pool()
            self._instance = await pool.acquire()
            return Redis(self._instance)
    
        async def __aexit__(self, exc_type, exc_val, exc_tb):
            pool = await self.get_pool()
            pool.release(self._instance)
            self._instance = None
    
        async def get_pool(self) -> ConnectionsPool:
            if not self._pool:
                self._pool = await aioredis.create_pool(
                    (config.host, config.port),
                    minsize=5,
                    maxsize=10
                )
    
            return self._pool

again, from my initial tests it seems to work better than the previous implementation (and also avoids using quit which, from the comments, is a bad thing to do with a pool. However, any changes I'll reflect in a later comment