fix: use AsyncSentinel for async Sentinel connections

nkanu17 · nkanu17 · commit 91b7c8e911d3 · 2026-03-03T14:25:37.000-05:00
Fixes #465 - Async Sentinel connections were using sync SentinelConnectionPool The _redis_sentinel_client() method was only using the sync Sentinel class, which caused AsyncRedis clients to be backed by sync SentinelConnectionPool. This resulted in runtime failures when async operations were attempted. Changes: - Import AsyncSentinel from redis.asyncio.sentinel - Branch on redis_class to use AsyncSentinel for async clients - Sync clients continue to use the sync Sentinel class - Added comprehensive docstrings for _redis_sentinel_client() and _parse_sentinel_url() - Improved type hints for _parse_sentinel_url() return type - Added edge case tests for Sentinel URL parsing - Added module docstring to test file - Updated docs with async Sentinel example - Fixed Python 3.9 compatibility in tests
diff --git 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
@@ -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
@@ -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
