diff --git a/docs/user_guide/installation.md b/docs/user_guide/installation.md index a2cc53a2..75431772 100644 --- a/docs/user_guide/installation.md +++ b/docs/user_guide/installation.md @@ -149,19 +149,25 @@ If you are considering a self-hosted Redis Enterprise deployment on Kubernetes, ### Redis Sentinel -For high availability deployments, RedisVL supports connecting to Redis through Sentinel. Use the `redis+sentinel://` URL scheme to connect: +For high availability deployments, RedisVL supports connecting to Redis through Sentinel. Use the `redis+sentinel://` URL scheme to connect. Both sync and async connections are fully supported. ```python -from redisvl.index import SearchIndex +from redisvl.index import SearchIndex, AsyncSearchIndex -# Connect via Sentinel +# Sync connection via Sentinel # Format: redis+sentinel://[username:password@]host1:port1,host2:port2/service_name[/db] index = SearchIndex.from_yaml( "schema.yaml", redis_url="redis+sentinel://sentinel1:26379,sentinel2:26379/mymaster" ) -# With authentication +# Async connection via Sentinel +async_index = AsyncSearchIndex.from_yaml( + "schema.yaml", + redis_url="redis+sentinel://sentinel1:26379,sentinel2:26379/mymaster" +) + +# With authentication and database selection index = SearchIndex.from_yaml( "schema.yaml", redis_url="redis+sentinel://user:pass@sentinel1:26379,sentinel2:26379/mymaster/0" @@ -172,5 +178,6 @@ The Sentinel URL format supports: - Multiple sentinel hosts (comma-separated) - Optional authentication (username:password) -- Service name (required - the name of the Redis master) +- Service name (defaults to `mymaster` if not specified) - Optional database number (defaults to 0) +- Both sync (`SearchIndex`) and async (`AsyncSearchIndex`) connections diff --git a/redisvl/redis/connection.py b/redisvl/redis/connection.py index bb071549..b10fd35c 100644 --- a/redisvl/redis/connection.py +++ b/redisvl/redis/connection.py @@ -10,6 +10,7 @@ from redis.asyncio.connection import AbstractConnection as AsyncAbstractConnection from redis.asyncio.connection import Connection as AsyncConnection from redis.asyncio.connection import SSLConnection as AsyncSSLConnection +from redis.asyncio.sentinel import Sentinel as AsyncSentinel from redis.connection import SSLConnection from redis.exceptions import ResponseError from redis.sentinel import Sentinel @@ -744,11 +745,33 @@ def _redis_sentinel_client( def _redis_sentinel_client( redis_url: str, redis_class: Union[type[Redis], type[AsyncRedis]], **kwargs: Any ) -> Union[Redis, AsyncRedis]: + """Create a Redis client connected via Sentinel for high availability. + + Parses a Sentinel URL and creates a Redis client connected to the + master instance discovered by Sentinel. Supports both sync and async + clients by using the appropriate Sentinel class. + + Args: + redis_url: Sentinel URL in the format: + ``redis+sentinel://[user:pass@]host1:port1[,host2:port2,...][/service][/db]`` + Service name defaults to "mymaster" if not specified. + redis_class: The Redis client class to use (Redis or AsyncRedis). + **kwargs: Additional arguments passed to Sentinel and master_for(). + + Returns: + A Redis client (sync or async) connected to the Sentinel-managed master. + + Example: + >>> client = RedisConnectionFactory._redis_sentinel_client( + ... "redis+sentinel://sentinel1:26379,sentinel2:26379/mymaster", + ... Redis + ... ) + """ sentinel_list, service_name, db, username, password = ( RedisConnectionFactory._parse_sentinel_url(redis_url) ) - sentinel_kwargs = {} + sentinel_kwargs: Dict[str, Any] = {} if username: sentinel_kwargs["username"] = username kwargs["username"] = username @@ -758,11 +781,46 @@ def _redis_sentinel_client( if db: kwargs["db"] = db - sentinel = Sentinel(sentinel_list, sentinel_kwargs=sentinel_kwargs, **kwargs) - return sentinel.master_for(service_name, redis_class=redis_class, **kwargs) + # Use AsyncSentinel for async clients, Sentinel for sync clients + if redis_class == AsyncRedis: + async_sentinel = AsyncSentinel( + sentinel_list, sentinel_kwargs=sentinel_kwargs, **kwargs + ) + return async_sentinel.master_for( + service_name, redis_class=redis_class, **kwargs # type: ignore[arg-type] + ) + else: + sync_sentinel = Sentinel( + sentinel_list, sentinel_kwargs=sentinel_kwargs, **kwargs + ) + return sync_sentinel.master_for( + service_name, redis_class=redis_class, **kwargs + ) @staticmethod - def _parse_sentinel_url(url: str) -> tuple: + def _parse_sentinel_url( + url: str, + ) -> Tuple[List[Tuple[str, int]], str, Optional[str], Optional[str], Optional[str]]: + """Parse a Redis Sentinel URL into its components. + + Args: + url: Sentinel URL in the format: + ``redis+sentinel://[user:pass@]host1:port1[,host2:port2,...]/service[/db]`` + + Returns: + A tuple containing: + - sentinel_list: List of (host, port) tuples for Sentinel nodes + - service_name: The Sentinel service name (defaults to "mymaster") + - db: The database number (or None if not specified) + - username: The username for authentication (or None) + - password: The password for authentication (or None) + + Example: + >>> RedisConnectionFactory._parse_sentinel_url( + ... "redis+sentinel://user:pass@host1:26379,host2:26380/mymaster/0" + ... ) + ([('host1', 26379), ('host2', 26380)], 'mymaster', '0', 'user', 'pass') + """ parsed_url = urlparse(url) hosts_part = parsed_url.netloc.split("@")[-1] sentinel_hosts = hosts_part.split(",") diff --git a/tests/unit/test_sentinel_url.py b/tests/unit/test_sentinel_url.py index 8c564eb8..e6687a2d 100644 --- a/tests/unit/test_sentinel_url.py +++ b/tests/unit/test_sentinel_url.py @@ -1,3 +1,19 @@ +"""Tests for Redis Sentinel URL connection handling. + +This module tests the RedisConnectionFactory's ability to create Redis clients +from Sentinel URLs (redis+sentinel://...). It verifies: + +1. Correct Sentinel class selection (AsyncSentinel for async, Sentinel for sync) +2. URL parsing (hosts, ports, service name, database, authentication) +3. Proper kwargs passthrough to Sentinel and master_for() +4. Error handling for connection failures + +These tests use mocking to avoid requiring a real Sentinel deployment. + +Related: GitHub Issue #465 - Async Sentinel connections were incorrectly using +the sync SentinelConnectionPool, causing runtime failures. +""" + from unittest.mock import MagicMock, patch import pytest @@ -12,7 +28,14 @@ def test_sentinel_url_connection(use_async): "redis+sentinel://username:password@host1:26379,host2:26380/mymaster/0" ) - with patch("redisvl.redis.connection.Sentinel") as mock_sentinel: + # Use appropriate Sentinel class based on sync/async + sentinel_patch_target = ( + "redisvl.redis.connection.AsyncSentinel" + if use_async + else "redisvl.redis.connection.Sentinel" + ) + + with patch(sentinel_patch_target) as mock_sentinel: mock_master = MagicMock() mock_sentinel.return_value.master_for.return_value = mock_master @@ -42,7 +65,14 @@ def test_sentinel_url_connection(use_async): def test_sentinel_url_connection_no_auth_no_db(use_async): sentinel_url = "redis+sentinel://host1:26379,host2:26380/mymaster" - with patch("redisvl.redis.connection.Sentinel") as mock_sentinel: + # Use appropriate Sentinel class based on sync/async + sentinel_patch_target = ( + "redisvl.redis.connection.AsyncSentinel" + if use_async + else "redisvl.redis.connection.Sentinel" + ) + + with patch(sentinel_patch_target) as mock_sentinel: mock_master = MagicMock() mock_sentinel.return_value.master_for.return_value = mock_master @@ -72,7 +102,14 @@ def test_sentinel_url_connection_no_auth_no_db(use_async): def test_sentinel_url_connection_error(use_async): sentinel_url = "redis+sentinel://host1:26379,host2:26380/mymaster" - with patch("redisvl.redis.connection.Sentinel") as mock_sentinel: + # Use appropriate Sentinel class based on sync/async + sentinel_patch_target = ( + "redisvl.redis.connection.AsyncSentinel" + if use_async + else "redisvl.redis.connection.Sentinel" + ) + + with patch(sentinel_patch_target) as mock_sentinel: mock_sentinel.return_value.master_for.side_effect = ConnectionError( "Test connection error" ) @@ -85,3 +122,137 @@ def test_sentinel_url_connection_error(use_async): RedisConnectionFactory.get_redis_connection(sentinel_url) mock_sentinel.assert_called_once() + + +def test_async_sentinel_uses_async_sentinel_class(): + """Test that async connections use AsyncSentinel (fix for issue #465).""" + sentinel_url = "redis+sentinel://host1:26379/mymaster" + + # Track which Sentinel class is called + sync_sentinel_called = False + async_sentinel_called = False + + def track_sync_sentinel(*args, **kwargs): + nonlocal sync_sentinel_called + sync_sentinel_called = True + mock = MagicMock() + mock.master_for.return_value = MagicMock() + return mock + + def track_async_sentinel(*args, **kwargs): + nonlocal async_sentinel_called + async_sentinel_called = True + mock = MagicMock() + mock.master_for.return_value = MagicMock() + return mock + + with ( + patch("redisvl.redis.connection.Sentinel", side_effect=track_sync_sentinel), + patch( + "redisvl.redis.connection.AsyncSentinel", side_effect=track_async_sentinel + ), + ): + with pytest.warns(DeprecationWarning): + RedisConnectionFactory.get_async_redis_connection(sentinel_url) + + # Verify AsyncSentinel was called, not sync Sentinel + assert async_sentinel_called, "AsyncSentinel should be called for async connections" + assert ( + not sync_sentinel_called + ), "Sync Sentinel should NOT be called for async connections" + + +def test_sync_sentinel_uses_sync_sentinel_class(): + """Test that sync connections use sync Sentinel.""" + sentinel_url = "redis+sentinel://host1:26379/mymaster" + + # Track which Sentinel class is called + sync_sentinel_called = False + async_sentinel_called = False + + def track_sync_sentinel(*args, **kwargs): + nonlocal sync_sentinel_called + sync_sentinel_called = True + mock = MagicMock() + mock.master_for.return_value = MagicMock() + return mock + + def track_async_sentinel(*args, **kwargs): + nonlocal async_sentinel_called + async_sentinel_called = True + mock = MagicMock() + mock.master_for.return_value = MagicMock() + return mock + + with ( + patch("redisvl.redis.connection.Sentinel", side_effect=track_sync_sentinel), + patch( + "redisvl.redis.connection.AsyncSentinel", side_effect=track_async_sentinel + ), + ): + RedisConnectionFactory.get_redis_connection(sentinel_url) + + # Verify sync Sentinel was called, not AsyncSentinel + assert sync_sentinel_called, "Sync Sentinel should be called for sync connections" + assert ( + not async_sentinel_called + ), "AsyncSentinel should NOT be called for sync connections" + + +# ============================================================================= +# Additional Edge Case Tests for Sentinel URL Parsing +# ============================================================================= + + +class TestSentinelUrlParsingEdgeCases: + """Tests for Sentinel URL parsing edge cases not covered by main tests.""" + + def test_sentinel_url_default_port_when_not_specified(self): + """Verify default port 26379 is used when port is omitted.""" + sentinel_url = "redis+sentinel://host1/mymaster" + + with patch("redisvl.redis.connection.Sentinel") as mock_sentinel: + mock_sentinel.return_value.master_for.return_value = MagicMock() + RedisConnectionFactory.get_redis_connection(sentinel_url) + + call_args = mock_sentinel.call_args + assert call_args[0][0] == [("host1", 26379)] + + def test_sentinel_url_default_service_name_when_path_empty(self): + """Verify default service name 'mymaster' when path is empty.""" + sentinel_url = "redis+sentinel://host1:26379" + + with patch("redisvl.redis.connection.Sentinel") as mock_sentinel: + mock_sentinel.return_value.master_for.return_value = MagicMock() + RedisConnectionFactory.get_redis_connection(sentinel_url) + + master_for_args = mock_sentinel.return_value.master_for.call_args + assert master_for_args[0][0] == "mymaster" + + def test_sentinel_url_password_only_auth(self): + """Verify password-only auth works (empty username).""" + sentinel_url = "redis+sentinel://:secretpass@host1:26379/mymaster" + + with patch("redisvl.redis.connection.Sentinel") as mock_sentinel: + mock_sentinel.return_value.master_for.return_value = MagicMock() + RedisConnectionFactory.get_redis_connection(sentinel_url) + + call_kwargs = mock_sentinel.call_args[1] + assert call_kwargs["sentinel_kwargs"]["password"] == "secretpass" + assert call_kwargs["password"] == "secretpass" + + def test_sentinel_custom_kwargs_passed_to_master_for(self): + """Verify custom kwargs are passed through to master_for call.""" + sentinel_url = "redis+sentinel://host1:26379/mymaster" + + with patch("redisvl.redis.connection.AsyncSentinel") as mock_async_sentinel: + mock_async_sentinel.return_value.master_for.return_value = MagicMock() + + with pytest.warns(DeprecationWarning): + RedisConnectionFactory.get_async_redis_connection( + sentinel_url, decode_responses=True, socket_timeout=5.0 + ) + + master_for_kwargs = mock_async_sentinel.return_value.master_for.call_args[1] + assert master_for_kwargs["decode_responses"] is True + assert master_for_kwargs["socket_timeout"] == 5.0 diff --git a/uv.lock b/uv.lock index 34277f94..0c567711 100644 --- a/uv.lock +++ b/uv.lock @@ -4771,7 +4771,7 @@ wheels = [ [[package]] name = "redisvl" -version = "0.14.1" +version = "0.15.0" source = { editable = "." } dependencies = [ { name = "jsonpath-ng" },