feat(cache): add warnings when using sync methods with async-only Redis client #390
Conversation
…ed with get method of embedding cache when async redis client is provided
- Add concise warning messages when sync methods are called with async-only client - Implement warning flag to prevent log spam (show warning only once per session) - Add comprehensive test coverage for warning behavior - Improve developer experience by providing clear guidance on method usage The warnings now clearly indicate which async methods should be used instead of sync methods when EmbeddingsCache is initialized with only an async client. This helps prevent confusion when developers unintentionally use different Redis connections than expected.
There was a problem hiding this comment.
Pull Request Overview
This PR adds warning functionality to detect when sync methods are called on EmbeddingsCache instances that only have an async Redis client configured. It improves developer experience by providing clear guidance when incorrect method types are used.
- Implements a class-level warning flag to prevent warning spam from repeated calls
- Adds warning logic to sync methods when only async client is configured
- Includes comprehensive test coverage for warning behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| redisvl/extensions/cache/embeddings/embeddings.py | Adds _warning_shown flag and warning logic to sync methods (get_by_key, mget_by_keys, set, mset) |
| tests/integration/test_embedcache_warnings.py | New test file covering warning behavior for sync/async method usage patterns |
Comments suppressed due to low confidence (1)
redisvl/extensions/cache/embeddings/embeddings.py:1
- The warning messages follow a similar pattern but are hardcoded in each method. Consider creating a helper method that takes the sync and async method names as parameters to generate consistent warning messages and reduce duplication.
"""Embeddings cache implementation for RedisVL."""
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| embedding_data = cache.get_by_key("embedcache:1234567890abcdef") | ||
| """ | ||
| if self._owns_redis_client is False and self._redis_client is None: |
There was a problem hiding this comment.
The warning condition self._owns_redis_client is False and self._redis_client is None is duplicated across four methods. Consider extracting this logic into a private helper method like _should_warn_async_only() to improve maintainability.
Copilot uses AI. Check for mistakes.
| if not keys: | ||
| return [] | ||
|
|
||
| if self._owns_redis_client is False and self._redis_client is None: |
There was a problem hiding this comment.
The warning condition self._owns_redis_client is False and self._redis_client is None is duplicated across four methods. Consider extracting this logic into a private helper method like _should_warn_async_only() to improve maintainability.
Copilot uses AI. Check for mistakes.
| text, model_name, embedding, metadata | ||
| ) | ||
|
|
||
| if self._owns_redis_client is False and self._redis_client is None: |
There was a problem hiding this comment.
The warning condition self._owns_redis_client is False and self._redis_client is None is duplicated across four methods. Consider extracting this logic into a private helper method like _should_warn_async_only() to improve maintainability.
Copilot uses AI. Check for mistakes.
| if not items: | ||
| return [] | ||
|
|
||
| if self._owns_redis_client is False and self._redis_client is None: |
There was a problem hiding this comment.
The warning condition self._owns_redis_client is False and self._redis_client is None is duplicated across four methods. Consider extracting this logic into a private helper method like _should_warn_async_only() to improve maintainability.
Copilot uses AI. Check for mistakes.
…ests - Convert sync test functions to async when using async Redis clients - Use _get_aredis_connection() for proper async client creation - Add proper try/finally blocks to ensure client cleanup - Fix 'Event loop is closed' errors in CI The issue was that async Redis clients were being created in sync test functions without proper event loop management. This caused cleanup issues when the test finished.
Improves upon #369