From bd89ee0a2422cc745cbae6ce0420662aaadd7dc9 Mon Sep 17 00:00:00 2001 From: Tyler Hutcherson Date: Tue, 1 Oct 2024 16:11:19 -0400 Subject: [PATCH 1/5] fix module version check for from_existing --- redisvl/index/index.py | 10 ++++++++-- tests/integration/test_connection.py | 12 +++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/redisvl/index/index.py b/redisvl/index/index.py index b4adb6b3..7e719873 100644 --- a/redisvl/index/index.py +++ b/redisvl/index/index.py @@ -354,7 +354,10 @@ def from_existing( # Validate modules installed_modules = RedisConnectionFactory.get_modules(redis_client) - validate_modules(installed_modules, [{"name": "search", "ver": 20810}]) + validate_modules( + installed_modules, + [{"name": "search", "ver": 20810}, {"name": "searchlight", "ver": 20810}] + ) # Fetch index info and convert to schema index_info = cls._info(name, redis_client) @@ -860,7 +863,10 @@ async def from_existing( # Validate modules installed_modules = await RedisConnectionFactory.get_modules_async(redis_client) - validate_modules(installed_modules, [{"name": "search", "ver": 20810}]) + validate_modules( + installed_modules, + [{"name": "search", "ver": 20810}, {"name": "searchlight", "ver": 20810}] + ) # Fetch index info and convert to schema index_info = await cls._info(name, redis_client) diff --git a/tests/integration/test_connection.py b/tests/integration/test_connection.py index 9a6035f2..5f63ee10 100644 --- a/tests/integration/test_connection.py +++ b/tests/integration/test_connection.py @@ -102,7 +102,7 @@ def test_convert_index_info_to_schema(): assert schema.index.name == index_info["index_name"] -def test_validate_modules_exist(): +def test_validate_modules_exist_search(): validate_modules( installed_modules={"search": 20811}, required_modules=[ @@ -112,6 +112,16 @@ def test_validate_modules_exist(): ) +def test_validate_modules_exist_searchlight(): + validate_modules( + installed_modules={"searchlight": 20819}, + required_modules=[ + {"name": "search", "ver": 20810}, + {"name": "searchlight", "ver": 20810}, + ], + ) + + def test_validate_modules_not_exist(): with pytest.raises(ValueError): validate_modules( From 56a3a65935726816028454fd29e101d68b9d0688 Mon Sep 17 00:00:00 2001 From: Tyler Hutcherson Date: Tue, 1 Oct 2024 16:42:36 -0400 Subject: [PATCH 2/5] add exception class for invalid modules --- redisvl/exceptions.py | 6 ++++++ redisvl/index/index.py | 30 ++++++++++++++++++++-------- redisvl/redis/connection.py | 14 ++++++++++--- tests/integration/test_connection.py | 3 ++- 4 files changed, 41 insertions(+), 12 deletions(-) create mode 100644 redisvl/exceptions.py diff --git a/redisvl/exceptions.py b/redisvl/exceptions.py new file mode 100644 index 00000000..79b165e3 --- /dev/null +++ b/redisvl/exceptions.py @@ -0,0 +1,6 @@ +class RedisVLException(Exception): + """Base RedisVL exception""" + + +class RedisModuleVersionError(RedisVLException): + """Invalid module versions installed""" diff --git a/redisvl/index/index.py b/redisvl/index/index.py index 7e719873..e5cf1668 100644 --- a/redisvl/index/index.py +++ b/redisvl/index/index.py @@ -25,6 +25,7 @@ import redis.asyncio as aredis from redis.commands.search.indexDefinition import IndexDefinition +from redisvl.exceptions import RedisModuleVersionError from redisvl.index.storage import BaseStorage, HashStorage, JsonStorage from redisvl.query import BaseQuery, CountQuery, FilterQuery from redisvl.query.filter import FilterExpression @@ -354,10 +355,17 @@ def from_existing( # Validate modules installed_modules = RedisConnectionFactory.get_modules(redis_client) - validate_modules( - installed_modules, - [{"name": "search", "ver": 20810}, {"name": "searchlight", "ver": 20810}] - ) + + try: + required_modules = [ + {"name": "search", "ver": 20810}, + {"name": "searchlight", "ver": 20810}, + ] + validate_modules(installed_modules, required_modules) + except RedisModuleVersionError as e: + raise RedisModuleVersionError( + f"Loading from existing index failed. {str(e)}" + ) # Fetch index info and convert to schema index_info = cls._info(name, redis_client) @@ -863,10 +871,16 @@ async def from_existing( # Validate modules installed_modules = await RedisConnectionFactory.get_modules_async(redis_client) - validate_modules( - installed_modules, - [{"name": "search", "ver": 20810}, {"name": "searchlight", "ver": 20810}] - ) + try: + required_modules = [ + {"name": "search", "ver": 20810}, + {"name": "searchlight", "ver": 20810}, + ] + validate_modules(installed_modules, required_modules) + except RedisModuleVersionError as e: + raise RedisModuleVersionError( + f"Loading from existing index failed. {str(e)}" + ) # Fetch index info and convert to schema index_info = await cls._info(name, redis_client) diff --git a/redisvl/redis/connection.py b/redisvl/redis/connection.py index 21095cde..27e3524b 100644 --- a/redisvl/redis/connection.py +++ b/redisvl/redis/connection.py @@ -9,6 +9,7 @@ from redis.connection import AbstractConnection, SSLConnection from redis.exceptions import ResponseError +from redisvl.exceptions import RedisModuleVersionError from redisvl.redis.constants import DEFAULT_REQUIRED_MODULES from redisvl.redis.utils import convert_bytes from redisvl.version import __version__ @@ -143,11 +144,18 @@ def validate_modules( if int(installed_version) >= int(required_module["ver"]): # type: ignore return - raise ValueError( - f"Required Redis database module {required_module['name']} with version >= {required_module['ver']} not installed. " - "See Redis Stack documentation: https://redis.io/docs/stack/" + error_message = "Required Redis db module " + + error_message += " OR ".join( + [f'{module["name"]} >= {module["ver"]}' for module in required_modules] + ) + + error_message += ( + " not installed. See Redis Stack docs at https://redis.io/docs/stack/." ) + raise RedisModuleVersionError(error_message) + class RedisConnectionFactory: """Builds connections to a Redis database, supporting both synchronous and diff --git a/tests/integration/test_connection.py b/tests/integration/test_connection.py index 5f63ee10..80556cdb 100644 --- a/tests/integration/test_connection.py +++ b/tests/integration/test_connection.py @@ -5,6 +5,7 @@ from redis.asyncio import Redis as AsyncRedis from redis.exceptions import ConnectionError +from redisvl.exceptions import RedisModuleVersionError from redisvl.redis.connection import ( RedisConnectionFactory, compare_versions, @@ -123,7 +124,7 @@ def test_validate_modules_exist_searchlight(): def test_validate_modules_not_exist(): - with pytest.raises(ValueError): + with pytest.raises(RedisModuleVersionError): validate_modules( installed_modules={"search": 20811}, required_modules=[ From fd5d73a4e2d44235dec0ea9397838848f6a8720b Mon Sep 17 00:00:00 2001 From: Tyler Hutcherson Date: Tue, 1 Oct 2024 22:04:45 -0400 Subject: [PATCH 3/5] fix test --- tests/integration/test_llmcache.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_llmcache.py b/tests/integration/test_llmcache.py index 4eb4d6f7..2a83bbf7 100644 --- a/tests/integration/test_llmcache.py +++ b/tests/integration/test_llmcache.py @@ -7,6 +7,7 @@ from pydantic.v1 import ValidationError from redis.exceptions import ConnectionError +from redisvl.exceptions import RedisModuleVersionError from redisvl.extensions.llmcache import SemanticCache from redisvl.index.index import AsyncSearchIndex, SearchIndex from redisvl.query.filter import Num, Tag, Text @@ -782,7 +783,8 @@ def test_index_updating(redis_url): ) assert response == [] - with pytest.raises(ValueError): + with pytest.raises((RedisModuleVersionError, ValueError)): + cache_with_tags = SemanticCache( name="test_cache", redis_url=redis_url, From 094600d95d00b327c91b71e39e309cf638abf5e0 Mon Sep 17 00:00:00 2001 From: Tyler Hutcherson Date: Wed, 2 Oct 2024 09:30:15 -0400 Subject: [PATCH 4/5] adjust error message and exception chain --- redisvl/index/index.py | 3 ++- redisvl/redis/connection.py | 11 +++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/redisvl/index/index.py b/redisvl/index/index.py index e5cf1668..0c4d3e2a 100644 --- a/redisvl/index/index.py +++ b/redisvl/index/index.py @@ -871,6 +871,7 @@ async def from_existing( # Validate modules installed_modules = await RedisConnectionFactory.get_modules_async(redis_client) + try: required_modules = [ {"name": "search", "ver": 20810}, @@ -880,7 +881,7 @@ async def from_existing( except RedisModuleVersionError as e: raise RedisModuleVersionError( f"Loading from existing index failed. {str(e)}" - ) + ) from e # Fetch index info and convert to schema index_info = await cls._info(name, redis_client) diff --git a/redisvl/redis/connection.py b/redisvl/redis/connection.py index 27e3524b..560004c0 100644 --- a/redisvl/redis/connection.py +++ b/redisvl/redis/connection.py @@ -144,14 +144,13 @@ def validate_modules( if int(installed_version) >= int(required_module["ver"]): # type: ignore return - error_message = "Required Redis db module " - - error_message += " OR ".join( + # Build the error message dynamically + required_modules_str = " OR ".join( [f'{module["name"]} >= {module["ver"]}' for module in required_modules] ) - - error_message += ( - " not installed. See Redis Stack docs at https://redis.io/docs/stack/." + error_message = ( + f"Required Redis db module {required_modules_str} not installed. " + "See Redis Stack docs at https://redis.io/docs/latest/operate/oss_and_stack/install/install-stack/." ) raise RedisModuleVersionError(error_message) From bbb22b39b386fd319c74f04d83245b2a698d3e6c Mon Sep 17 00:00:00 2001 From: Tyler Hutcherson Date: Wed, 2 Oct 2024 10:27:33 -0400 Subject: [PATCH 5/5] use new exception --- tests/integration/test_llmcache.py | 3 ++- tests/integration/test_semantic_router.py | 4 +++- tests/integration/test_session_manager.py | 4 +++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_llmcache.py b/tests/integration/test_llmcache.py index 2a83bbf7..c2b80e8a 100644 --- a/tests/integration/test_llmcache.py +++ b/tests/integration/test_llmcache.py @@ -872,7 +872,8 @@ def test_bad_dtype_connecting_to_existing_cache(): try: cache = SemanticCache(name="float64_cache", dtype="float64") same_type = SemanticCache(name="float64_cache", dtype="float64") - except ValueError: + # under the hood uses from_existing + except RedisModuleVersionError: pytest.skip("Not using a late enough version of Redis") with pytest.raises(ValueError): diff --git a/tests/integration/test_semantic_router.py b/tests/integration/test_semantic_router.py index 194a6f98..8c1f6cf8 100644 --- a/tests/integration/test_semantic_router.py +++ b/tests/integration/test_semantic_router.py @@ -4,6 +4,7 @@ import pytest from redis.exceptions import ConnectionError +from redisvl.exceptions import RedisModuleVersionError from redisvl.extensions.router import SemanticRouter from redisvl.extensions.router.schema import Route, RoutingConfig from redisvl.redis.connection import compare_versions @@ -285,7 +286,8 @@ def test_bad_dtype_connecting_to_exiting_router(routes): routes=routes, dtype="float64", ) - except ValueError: + # under the hood uses from_existing + except RedisModuleVersionError: pytest.skip("Not using a late enough version of Redis") with pytest.raises(ValueError): diff --git a/tests/integration/test_session_manager.py b/tests/integration/test_session_manager.py index 898c3ab5..54267c54 100644 --- a/tests/integration/test_session_manager.py +++ b/tests/integration/test_session_manager.py @@ -3,6 +3,7 @@ import pytest from redis.exceptions import ConnectionError +from redisvl.exceptions import RedisModuleVersionError from redisvl.extensions.constants import ID_FIELD_NAME from redisvl.extensions.session_manager import ( SemanticSessionManager, @@ -566,7 +567,8 @@ def test_bad_dtype_connecting_to_exiting_session(): try: session = SemanticSessionManager(name="float64 session", dtype="float64") same_type = SemanticSessionManager(name="float64 session", dtype="float64") - except ValueError: + # under the hood uses from_existing + except RedisModuleVersionError: pytest.skip("Not using a late enough version of Redis") with pytest.raises(ValueError):