diff --git a/python/semantic_kernel/connectors/redis.py b/python/semantic_kernel/connectors/redis.py index 575624895aca..5933aeba2097 100644 --- a/python/semantic_kernel/connectors/redis.py +++ b/python/semantic_kernel/connectors/redis.py @@ -17,6 +17,7 @@ from redis.commands.search.field import Field as RedisField from redis.commands.search.field import NumericField, TagField, TextField, VectorField from redis.commands.search.index_definition import IndexDefinition, IndexType +from redis.exceptions import ResponseError from redisvl.index.index import process_results from redisvl.query.filter import FilterExpression, Num, Tag, Text from redisvl.query.query import BaseQuery, VectorQuery @@ -287,7 +288,13 @@ async def collection_exists(self, **kwargs) -> bool: try: await self.redis_database.ft(self.collection_name).info() return True - except Exception: + except ResponseError: + # Redis answers FT.INFO for an unknown index with an error reply, which redis-py + # surfaces as ResponseError. ConnectionError, TimeoutError and AuthenticationError are + # siblings of ResponseError rather than subclasses, so they now propagate instead of + # being reported as "the index is not there". ensure_collection_deleted() gates the + # drop on this answer, so a swallowed timeout made it return successfully having + # deleted nothing. return False @override diff --git a/python/tests/unit/connectors/memory/test_redis_store.py b/python/tests/unit/connectors/memory/test_redis_store.py index e779ad945a97..a5a23da1209a 100644 --- a/python/tests/unit/connectors/memory/test_redis_store.py +++ b/python/tests/unit/connectors/memory/test_redis_store.py @@ -5,6 +5,8 @@ import numpy as np from pytest import fixture, mark, raises from redis.asyncio.client import Redis +from redis.exceptions import ConnectionError as RedisConnectionError +from redis.exceptions import ResponseError from semantic_kernel.connectors.redis import ( RedisCollectionTypes, @@ -281,11 +283,24 @@ async def test_collection_exists(collection_hash, mock_collection_exists): async def test_collection_exists_false(collection_hash, mock_collection_exists): - mock_collection_exists.side_effect = Exception + # ResponseError is what redis-py raises when FT.INFO reports an unknown index. The previous + # bare Exception did not represent that, and matched a timeout just as readily. + mock_collection_exists.side_effect = ResponseError("Unknown Index name") exists = await collection_hash.collection_exists() assert not exists +async def test_collection_exists_propagates_connection_errors(collection_hash, mock_collection_exists): + """A cluster that cannot be reached is not an absent index. + + ensure_collection_deleted() drops the index only when collection_exists() says it is there, so + reporting a connection failure as "not there" made the delete a silent no-op. + """ + mock_collection_exists.side_effect = RedisConnectionError("connection refused") + with raises(RedisConnectionError): + await collection_hash.collection_exists() + + async def test_ensure_collection_deleted(collection_hash, mock_ensure_collection_deleted): await collection_hash.ensure_collection_deleted() await collection_hash.ensure_collection_deleted()