channels_redis
channels_redis copied to clipboard
django
migrate aioredis to v2.0
This has been discussed in #268 and mentioned here.
Upgrading to 2.0 is the plan, just no one has done the work yet.
This library is now part of the mainstream Redis library as of 4.2.0-rc1 https://github.com/redis/redis-py/pull/1899
Oh, that is good!
Very happy to take a PR implementing this swap over.
I dug into this a bit, could you explain what the reasoning is for this?
https://github.com/django/channels_redis/blob/f5eef165017a702a1e53fe19a673036c0e337c95/channels_redis/core.py#L73-L79
It's creating a single connection pool then treating and referring to it as a connection. Some of the paradigms the library uses like checking if the pool is closed don't map in redis-py. You can check if a connection is closed and you can release all the connections in a pool, but you don't check if a pool is closed.
It was added for the sentinel support. 🤔 @qeternity — Can you comment on the reasoning there? Thanks.
Hi @ipmb - The API for pools and connections is very similar (at least in aioredis 1.3.x). We can't use a raw redis connection with sentinel because a sentinel connection is actually two redis connections: one pubsub connection to a sentinel, and then a connection to the actual redis instance. You need to listen to the pubsub connection for changes from the sentinel(s). This logic was all orchestrated automatically by the aioredis sentinel pool. However, because non-pubsub layer does not use the library pooling, and marshals connections manually, we don't want to risk letting the sentinel connections in the library pool grow, because we will end up having many pools. All of this was done to make sentinel drop-in compatible with the non-pubsub layer.
I have not looked at the new redis-py migration, but in the 1.3.x branch, you absolutely could check if a pool is closed: https://github.com/aio-libs/aioredis-py/blob/8a207609b7f8a33e74c7c8130d97186e78cc0052/aioredis/sentinel/pool.py#L160
Yeah, it's not available here https://github.com/redis/redis-py/blob/c5d19b8571d2b15a29637f56a51b0da560072945/redis/asyncio/connection.py#L1379-L1538
I'm guessing we can iterate over the connections ourselves and verify they have all been closed.
Yep, there were a lot of breaking changes in 2.x which is why we never migrated.
That said, the fix here should be to use pools instead. This library has basically reimplemented its own pool manager.
This was done in #296 — will be in 4.0. 4.0.0b2 is available for testing now.