Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix module version checks #227

Merged
merged 5 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions redisvl/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class RedisVLException(Exception):
"""Base RedisVL exception"""


class RedisModuleVersionError(RedisVLException):
"""Invalid module versions installed"""
25 changes: 23 additions & 2 deletions redisvl/index/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -354,7 +355,17 @@ def from_existing(

# Validate modules
installed_modules = RedisConnectionFactory.get_modules(redis_client)
validate_modules(installed_modules, [{"name": "search", "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)
Expand Down Expand Up @@ -860,7 +871,17 @@ async def from_existing(

# Validate modules
installed_modules = await RedisConnectionFactory.get_modules_async(redis_client)
validate_modules(installed_modules, [{"name": "search", "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)}"
) from e

# Fetch index info and convert to schema
index_info = await cls._info(name, redis_client)
Expand Down
13 changes: 10 additions & 3 deletions redisvl/redis/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__
Expand Down Expand Up @@ -143,10 +144,16 @@ 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/"
# Build the error message dynamically
required_modules_str = " OR ".join(
[f'{module["name"]} >= {module["ver"]}' for module in required_modules]
)
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)


class RedisConnectionFactory:
Expand Down
15 changes: 13 additions & 2 deletions tests/integration/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -102,7 +103,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=[
Expand All @@ -112,8 +113,18 @@ 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):
with pytest.raises(RedisModuleVersionError):
validate_modules(
installed_modules={"search": 20811},
required_modules=[
Expand Down
7 changes: 5 additions & 2 deletions tests/integration/test_llmcache.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -870,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):
Expand Down
4 changes: 3 additions & 1 deletion tests/integration/test_semantic_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
4 changes: 3 additions & 1 deletion tests/integration/test_session_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down
Loading