-
Notifications
You must be signed in to change notification settings - Fork 66
Open
Description
Description
The Sentinel URL support added in PR #385 has a bug in the async implementation. The _redis_sentinel_client() method uses the sync redis.sentinel.Sentinel class for both sync
and async clients. When creating an async client, this results in an AsyncRedis instance backed by a sync SentinelConnectionPool, which lacks async methods and will fail at
runtime.
Current Behavior
# redisvl/redis/connection.py
from redis.sentinel import Sentinel # Only sync Sentinel imported
@staticmethod
def _redis_sentinel_client(redis_url, redis_class, **kwargs):
# ...
sentinel = Sentinel(sentinel_list, sentinel_kwargs=sentinel_kwargs, **kwargs) # Sync Sentinel
return sentinel.master_for(service_name, redis_class=redis_class, **kwargs) # Mixed sync/async When redis_class=AsyncRedis, this creates:
- An
AsyncRedisclient - With a sync
SentinelConnectionPool(from redis.sentinel) - The sync pool inherits from
redis.connection.ConnectionPool, notredis.asyncio.connection.ConnectionPool - Missing async methods like
aconnect()
Reproduction
from redisvl.redis.connection import RedisConnectionFactory
# Get async client via Sentinel URL
client = RedisConnectionFactory.get_async_redis_connection(
"redis+sentinel://localhost:26379/mymaster"
)
# Check the connection pool type
print(f"Client type: {type(client)}")
# <class 'redis.asyncio.client.Redis'>
print(f"Pool type: {type(client.connection_pool)}")
# <class 'redis.sentinel.SentinelConnectionPool'> <-- WRONG! Should be async pool
print(f"Pool base: {type(client.connection_pool).__bases__}")
# (<class 'redis.connection.ConnectionPool'>,) <-- Sync base class!
# This will fail when actually used:
# await client.ping() # Will fail - sync pool can't handle async operations Expected Behavior
Async Sentinel connections should use redis.asyncio.sentinel.Sentinel which creates an AsyncSentinelConnectionPool:
from redis.sentinel import Sentinel
from redis.asyncio.sentinel import Sentinel as AsyncSentinel
from redis.asyncio import Redis as AsyncRedis
@staticmethod
def _redis_sentinel_client(redis_url, redis_class, **kwargs):
sentinel_list, service_name, db, username, password = (
RedisConnectionFactory._parse_sentinel_url(redis_url)
)
sentinel_kwargs = {}
if username:
sentinel_kwargs["username"] = username
kwargs["username"] = username
if password:
sentinel_kwargs["password"] = password
kwargs["password"] = password
if db:
kwargs["db"] = db
# Use appropriate Sentinel class based on redis_class
if redis_class == AsyncRedis:
sentinel = AsyncSentinel(sentinel_list, sentinel_kwargs=sentinel_kwargs, **kwargs)
else:
sentinel = Sentinel(sentinel_list, sentinel_kwargs=sentinel_kwargs, **kwargs)
return sentinel.master_for(service_name, redis_class=redis_class, **kwargs) Impact
- All async Sentinel connections via URL are broken
- Affects get_async_redis_connection() and _get_aredis_connection() with redis+sentinel:// URLs
- Downstream libraries (like langgraph-checkpoint-redis) that use async savers/stores with Sentinel URLs will fail
Environment
- redisvl version: 0.11.0+ (any version with Sentinel support from PR feat: add Redis Sentinel URL scheme support (#213) #385)
- redis-py version: 5.x
- Python: 3.9+
Workaround
Users can create their own Sentinel-managed client and pass it directly:
from redis.asyncio.sentinel import Sentinel as AsyncSentinel
from redisvl.index import AsyncSearchIndex
sentinel = AsyncSentinel([('sentinel1', 26379), ('sentinel2', 26379)])
master = sentinel.master_for('mymaster')
index = AsyncSearchIndex.from_dict(schema, redis_client=master) Reactions are currently unavailable