Skip to content

Commit 8642a83

Browse files
committed
perf: remove proactive module validation to reduce network calls (#370)
Remove MODULE LIST calls during connection initialization and index creation. Operations now fail naturally when Redis modules are missing, providing clear error messages at the point of failure. Changes: - Remove validate_modules() function and DEFAULT_REQUIRED_MODULES constant - Remove required_modules parameter from all connection methods - Remove proactive validation from SearchIndex and AsyncSearchIndex - Remove validation from SemanticRouter.from_existing() - Fix SemanticCache to use lazy client initialization - Remove skip_if_module_version_error test helper - Remove unused RedisModuleVersionError exception - Add RediSearch module availability checks to skip tests when module not present - Fix mocking strategy to patch instance methods instead of class methods - Handle different Redis versions (6.2.6-v9, latest, 8.0.2) in CI environment - Ensure tests work with both testcontainers (local) and provided Redis (CI) - Ensures compatibility across different Redis Stack versions - Redis 6.2.6-v9 returns 'dims' while other versions may return 'dim'.
1 parent b32ed95 commit 8642a83

File tree

13 files changed

+512
-250
lines changed

13 files changed

+512
-250
lines changed

redisvl/exceptions.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,6 @@ class RedisVLError(Exception):
1111
pass
1212

1313

14-
class RedisModuleVersionError(RedisVLError):
15-
"""Error raised when required Redis modules are missing or have incompatible versions."""
16-
17-
pass
18-
19-
2014
class RedisSearchError(RedisVLError):
2115
"""Error raised for Redis Search specific operations."""
2216

redisvl/extensions/cache/llm/semantic.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ def __init__(
162162
)
163163

164164
existing_index = SearchIndex.from_existing(
165-
name, redis_client=self._index.client
165+
name, redis_client=self._index._redis_client
166166
)
167167
if existing_index.schema.to_dict() != self._index.schema.to_dict():
168168
raise ValueError(

redisvl/extensions/router/semantic.py

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
from redis.commands.search.aggregation import AggregateRequest, AggregateResult, Reducer
88
from redis.exceptions import ResponseError
99

10-
from redisvl.exceptions import RedisModuleVersionError
1110
from redisvl.extensions.constants import ROUTE_VECTOR_FIELD_NAME
1211
from redisvl.extensions.router.schema import (
1312
DistanceAggregationMethod,
@@ -118,18 +117,14 @@ def from_existing(
118117
**kwargs,
119118
) -> "SemanticRouter":
120119
"""Return SemanticRouter instance from existing index."""
121-
try:
122-
if redis_url:
123-
redis_client = RedisConnectionFactory.get_redis_connection(
124-
redis_url=redis_url,
125-
**kwargs,
126-
)
127-
elif redis_client:
128-
RedisConnectionFactory.validate_sync_redis(redis_client)
129-
except RedisModuleVersionError as e:
130-
raise RedisModuleVersionError(
131-
f"Loading from existing index failed. {str(e)}"
120+
if redis_url:
121+
redis_client = RedisConnectionFactory.get_redis_connection(
122+
redis_url=redis_url,
123+
**kwargs,
132124
)
125+
elif redis_client:
126+
# Just validate client type and set lib name
127+
RedisConnectionFactory.validate_sync_redis(redis_client)
133128
if redis_client is None:
134129
raise ValueError(
135130
"Creating Redis client failed. Please check the redis_url and connection_kwargs."

redisvl/index/index.py

Lines changed: 28 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@
6767

6868
from redisvl.exceptions import (
6969
QueryValidationError,
70-
RedisModuleVersionError,
7170
RedisSearchError,
7271
RedisVLError,
7372
SchemaValidationError,
@@ -439,23 +438,15 @@ def from_existing(
439438
440439
Raises:
441440
ValueError: If redis_url or redis_client is not provided.
442-
RedisModuleVersionError: If required Redis modules are not installed.
443441
"""
444-
try:
445-
if redis_url:
446-
redis_client = RedisConnectionFactory.get_redis_connection(
447-
redis_url=redis_url,
448-
required_modules=REQUIRED_MODULES_FOR_INTROSPECTION,
449-
**kwargs,
450-
)
451-
elif redis_client:
452-
RedisConnectionFactory.validate_sync_redis(
453-
redis_client, required_modules=REQUIRED_MODULES_FOR_INTROSPECTION
454-
)
455-
except RedisModuleVersionError as e:
456-
raise RedisModuleVersionError(
457-
f"Loading from existing index failed. {str(e)}"
442+
if redis_url:
443+
redis_client = RedisConnectionFactory.get_redis_connection(
444+
redis_url=redis_url,
445+
**kwargs,
458446
)
447+
elif redis_client:
448+
# Validate client type and set lib name
449+
RedisConnectionFactory.validate_sync_redis(redis_client)
459450

460451
if not redis_client:
461452
raise ValueError("Must provide either a redis_url or redis_client")
@@ -481,11 +472,16 @@ def _redis_client(self) -> SyncRedisClient:
481472
if self.__redis_client is None:
482473
with self._lock:
483474
if self.__redis_client is None:
475+
# Pass lib_name to connection factory
476+
kwargs = {**self._connection_kwargs}
477+
if self._lib_name:
478+
kwargs["lib_name"] = self._lib_name
484479
self.__redis_client = RedisConnectionFactory.get_redis_connection(
485480
redis_url=self._redis_url,
486-
**self._connection_kwargs,
481+
**kwargs,
487482
)
488-
if not self._validated_client:
483+
if not self._validated_client and self._lib_name:
484+
# Only set lib name for user-provided clients
489485
RedisConnectionFactory.validate_sync_redis(
490486
self.__redis_client,
491487
self._lib_name,
@@ -1193,21 +1189,14 @@ async def from_existing(
11931189
"Must provide either a redis_url or redis_client to fetch Redis index info."
11941190
)
11951191

1196-
try:
1197-
if redis_url:
1198-
redis_client = await RedisConnectionFactory._get_aredis_connection(
1199-
url=redis_url,
1200-
required_modules=REQUIRED_MODULES_FOR_INTROSPECTION,
1201-
**kwargs,
1202-
)
1203-
elif redis_client:
1204-
await RedisConnectionFactory.validate_async_redis(
1205-
redis_client, required_modules=REQUIRED_MODULES_FOR_INTROSPECTION
1206-
)
1207-
except RedisModuleVersionError as e:
1208-
raise RedisModuleVersionError(
1209-
f"Loading from existing index failed. {str(e)}"
1210-
) from e
1192+
if redis_url:
1193+
redis_client = await RedisConnectionFactory._get_aredis_connection(
1194+
url=redis_url,
1195+
**kwargs,
1196+
)
1197+
elif redis_client:
1198+
# Validate client type and set lib name
1199+
await RedisConnectionFactory.validate_async_redis(redis_client)
12111200

12121201
if redis_client is None:
12131202
raise ValueError(
@@ -1256,13 +1245,17 @@ async def _get_client(self) -> AsyncRedisClient:
12561245
async with self._lock:
12571246
# Double-check to protect against concurrent access
12581247
if self._redis_client is None:
1259-
kwargs = self._connection_kwargs
1248+
# Pass lib_name to connection factory
1249+
kwargs = {**self._connection_kwargs}
12601250
if self._redis_url:
12611251
kwargs["url"] = self._redis_url
1252+
if self._lib_name:
1253+
kwargs["lib_name"] = self._lib_name
12621254
self._redis_client = (
12631255
await RedisConnectionFactory._get_aredis_connection(**kwargs)
12641256
)
1265-
if not self._validated_client:
1257+
if not self._validated_client and self._lib_name:
1258+
# Set lib name for user-provided clients
12661259
await RedisConnectionFactory.validate_async_redis(
12671260
self._redis_client,
12681261
self._lib_name,

redisvl/redis/connection.py

Lines changed: 36 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@
1313
from redis.exceptions import ResponseError
1414

1515
from redisvl import __version__
16-
from redisvl.exceptions import RedisModuleVersionError
17-
from redisvl.redis.constants import DEFAULT_REQUIRED_MODULES, REDIS_URL_ENV_VAR
16+
from redisvl.redis.constants import REDIS_URL_ENV_VAR
1817
from redisvl.redis.utils import convert_bytes, is_cluster_url
1918
from redisvl.types import AsyncRedisClient, RedisClient, SyncRedisClient
2019
from redisvl.utils.utils import deprecated_function
@@ -93,7 +92,11 @@ def convert_index_info_to_schema(index_info: Dict[str, Any]) -> Dict[str, Any]:
9392

9493
def parse_vector_attrs(attrs):
9594
vector_attrs = {attrs[i].lower(): attrs[i + 1] for i in range(6, len(attrs), 2)}
96-
vector_attrs["dims"] = int(vector_attrs.pop("dim"))
95+
# Handle both "dim" and "dims" for compatibility across Redis versions
96+
if "dim" in vector_attrs:
97+
vector_attrs["dims"] = int(vector_attrs.pop("dim"))
98+
elif "dims" in vector_attrs:
99+
vector_attrs["dims"] = int(vector_attrs["dims"])
97100
vector_attrs["distance_metric"] = vector_attrs.pop("distance_metric").lower()
98101
vector_attrs["algorithm"] = vector_attrs.pop("algorithm").lower()
99102
vector_attrs["datatype"] = vector_attrs.pop("data_type").lower()
@@ -155,40 +158,6 @@ def parse_attrs(attrs):
155158
}
156159

157160

158-
def validate_modules(
159-
installed_modules: Dict[str, Any],
160-
required_modules: Optional[List[Dict[str, Any]]] = None,
161-
) -> None:
162-
"""
163-
Validates if required Redis modules are installed.
164-
165-
Args:
166-
installed_modules: List of installed modules.
167-
required_modules: List of required modules.
168-
169-
Raises:
170-
RedisModuleVersionError: If required Redis modules are not installed.
171-
"""
172-
required_modules = required_modules or DEFAULT_REQUIRED_MODULES
173-
174-
for required_module in required_modules:
175-
if required_module["name"] in installed_modules:
176-
installed_version = installed_modules[required_module["name"]] # type: ignore
177-
if int(installed_version) >= int(required_module["ver"]): # type: ignore
178-
return
179-
180-
# Build the error message dynamically
181-
required_modules_str = " OR ".join(
182-
[f'{module["name"]} >= {module["ver"]}' for module in required_modules]
183-
)
184-
error_message = (
185-
f"Required Redis db module {required_modules_str} not installed. "
186-
"See Redis Stack docs at https://redis.io/docs/latest/operate/oss_and_stack/install/install-stack/."
187-
)
188-
189-
raise RedisModuleVersionError(error_message)
190-
191-
192161
class RedisConnectionFactory:
193162
"""Builds connections to a Redis database, supporting both synchronous and
194163
asynchronous clients.
@@ -232,16 +201,13 @@ def connect(
232201
@staticmethod
233202
def get_redis_connection(
234203
redis_url: Optional[str] = None,
235-
required_modules: Optional[List[Dict[str, Any]]] = None,
236204
**kwargs,
237205
) -> SyncRedisClient:
238206
"""Creates and returns a synchronous Redis client.
239207
240208
Args:
241209
url (Optional[str]): The URL of the Redis server. If not provided,
242210
the environment variable REDIS_URL is used.
243-
required_modules (Optional[List[Dict[str, Any]]]): List of required
244-
Redis modules with version requirements.
245211
**kwargs: Additional keyword arguments to be passed to the Redis
246212
client constructor.
247213
@@ -251,22 +217,26 @@ def get_redis_connection(
251217
Raises:
252218
ValueError: If url is not provided and REDIS_URL environment
253219
variable is not set.
254-
RedisModuleVersionError: If required Redis modules are not installed.
255220
"""
256221
url = redis_url or get_address_from_env()
257222
if is_cluster_url(url, **kwargs):
258223
client = RedisCluster.from_url(url, **kwargs)
259224
else:
260225
client = Redis.from_url(url, **kwargs)
261-
RedisConnectionFactory.validate_sync_redis(
262-
client, required_modules=required_modules
263-
)
226+
# Module validation removed - operations will fail naturally if modules are missing
227+
# Set client library name only
228+
_lib_name = make_lib_name(kwargs.get("lib_name"))
229+
try:
230+
client.client_setinfo("LIB-NAME", _lib_name)
231+
except ResponseError:
232+
# Fall back to a simple log echo
233+
if hasattr(client, "echo"):
234+
client.echo(_lib_name)
264235
return client
265236

266237
@staticmethod
267238
async def _get_aredis_connection(
268239
url: Optional[str] = None,
269-
required_modules: Optional[List[Dict[str, Any]]] = None,
270240
**kwargs,
271241
) -> AsyncRedisClient:
272242
"""Creates and returns an asynchronous Redis client.
@@ -277,8 +247,6 @@ async def _get_aredis_connection(
277247
Args:
278248
url (Optional[str]): The URL of the Redis server. If not provided,
279249
the environment variable REDIS_URL is used.
280-
required_modules (Optional[List[Dict[str, Any]]]): List of required
281-
Redis modules with version requirements.
282250
**kwargs: Additional keyword arguments to be passed to the async
283251
Redis client constructor.
284252
@@ -288,7 +256,6 @@ async def _get_aredis_connection(
288256
Raises:
289257
ValueError: If url is not provided and REDIS_URL environment
290258
variable is not set.
291-
RedisModuleVersionError: If required Redis modules are not installed.
292259
"""
293260
url = url or get_address_from_env()
294261

@@ -297,9 +264,15 @@ async def _get_aredis_connection(
297264
else:
298265
client = AsyncRedis.from_url(url, **kwargs)
299266

300-
await RedisConnectionFactory.validate_async_redis(
301-
client, required_modules=required_modules
302-
)
267+
# Module validation removed - operations will fail naturally if modules are missing
268+
# Set client library name only
269+
_lib_name = make_lib_name(kwargs.get("lib_name"))
270+
try:
271+
await client.client_setinfo("LIB-NAME", _lib_name)
272+
except ResponseError:
273+
# Fall back to a simple log echo
274+
if hasattr(client, "echo"):
275+
await client.echo(_lib_name)
303276
return client
304277

305278
@staticmethod
@@ -386,9 +359,12 @@ async def get_modules_async(client: AsyncRedisClient) -> Dict[str, Any]:
386359
def validate_sync_redis(
387360
redis_client: SyncRedisClient,
388361
lib_name: Optional[str] = None,
389-
required_modules: Optional[List[Dict[str, Any]]] = None,
390362
) -> None:
391-
"""Validates the sync Redis client."""
363+
"""Validates the sync Redis client.
364+
365+
Note: Module validation has been removed. This method now only validates
366+
the client type and sets the library name.
367+
"""
392368
if not issubclass(type(redis_client), (Redis, RedisCluster)):
393369
raise TypeError(
394370
"Invalid Redis client instance. Must be Redis or RedisCluster."
@@ -404,19 +380,18 @@ def validate_sync_redis(
404380
if hasattr(redis_client, "echo"):
405381
redis_client.echo(_lib_name)
406382

407-
# Get list of modules
408-
installed_modules = RedisConnectionFactory.get_modules(redis_client)
409-
410-
# Validate available modules
411-
validate_modules(installed_modules, required_modules)
383+
# Module validation removed - operations will fail naturally if modules are missing
412384

413385
@staticmethod
414386
async def validate_async_redis(
415387
redis_client: AsyncRedisClient,
416388
lib_name: Optional[str] = None,
417-
required_modules: Optional[List[Dict[str, Any]]] = None,
418389
) -> None:
419-
"""Validates the async Redis client."""
390+
"""Validates the async Redis client.
391+
392+
Note: Module validation has been removed. This method now only validates
393+
the client type and sets the library name.
394+
"""
420395
if not issubclass(type(redis_client), (AsyncRedis, AsyncRedisCluster)):
421396
raise TypeError(
422397
"Invalid async Redis client instance. Must be async Redis or async RedisCluster."
@@ -427,12 +402,7 @@ async def validate_async_redis(
427402
await redis_client.client_setinfo("LIB-NAME", _lib_name)
428403
except ResponseError:
429404
# Fall back to a simple log echo
430-
await redis_client.echo(_lib_name)
431405
if hasattr(redis_client, "echo"):
432406
await redis_client.echo(_lib_name)
433407

434-
# Get list of modules
435-
installed_modules = await RedisConnectionFactory.get_modules_async(redis_client)
436-
437-
# Validate available modules
438-
validate_modules(installed_modules, required_modules)
408+
# Module validation removed - operations will fail naturally if modules are missing

0 commit comments

Comments
 (0)