Skip to content

Commit a2fb8f1

Browse files
bsboddenGitHub Copilot
andcommitted
refactor: address Copilot review suggestions
- Extract duplicated condition check into _should_warn_for_async_only() helper method - Add pytest autouse fixture to reset warning flag before each test - Improves code maintainability and test isolation Co-authored-by: GitHub Copilot <[email protected]>
1 parent 8ad8351 commit a2fb8f1

File tree

2 files changed

+21
-13
lines changed

2 files changed

+21
-13
lines changed

redisvl/extensions/cache/embeddings/embeddings.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,14 @@ def _process_cache_data(
126126
cache_hit = CacheEntry(**convert_bytes(data))
127127
return cache_hit.model_dump(exclude_none=True)
128128

129+
def _should_warn_for_async_only(self) -> bool:
130+
"""Check if warning should be shown for async-only client usage.
131+
132+
Returns:
133+
bool: True if only async client is available and warning hasn't been shown.
134+
"""
135+
return self._owns_redis_client is False and self._redis_client is None
136+
129137
def get(
130138
self,
131139
text: str,
@@ -169,7 +177,7 @@ def get_by_key(self, key: str) -> Optional[Dict[str, Any]]:
169177
170178
embedding_data = cache.get_by_key("embedcache:1234567890abcdef")
171179
"""
172-
if self._owns_redis_client is False and self._redis_client is None:
180+
if self._should_warn_for_async_only():
173181
if not EmbeddingsCache._warning_shown:
174182
logger.warning(
175183
"EmbeddingsCache initialized with async_redis_client only. "
@@ -212,7 +220,7 @@ def mget_by_keys(self, keys: List[str]) -> List[Optional[Dict[str, Any]]]:
212220
if not keys:
213221
return []
214222

215-
if self._owns_redis_client is False and self._redis_client is None:
223+
if self._should_warn_for_async_only():
216224
if not EmbeddingsCache._warning_shown:
217225
logger.warning(
218226
"EmbeddingsCache initialized with async_redis_client only. "
@@ -301,7 +309,7 @@ def set(
301309
text, model_name, embedding, metadata
302310
)
303311

304-
if self._owns_redis_client is False and self._redis_client is None:
312+
if self._should_warn_for_async_only():
305313
if not EmbeddingsCache._warning_shown:
306314
logger.warning(
307315
"EmbeddingsCache initialized with async_redis_client only. "
@@ -359,7 +367,7 @@ def mset(
359367
if not items:
360368
return []
361369

362-
if self._owns_redis_client is False and self._redis_client is None:
370+
if self._should_warn_for_async_only():
363371
if not EmbeddingsCache._warning_shown:
364372
logger.warning(
365373
"EmbeddingsCache initialized with async_redis_client only. "

tests/integration/test_embedcache_warnings.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,18 @@
99
from redisvl.extensions.cache.embeddings import EmbeddingsCache
1010

1111

12+
@pytest.fixture(autouse=True)
13+
def reset_warning_flag():
14+
"""Reset the warning flag before each test to ensure test isolation."""
15+
EmbeddingsCache._warning_shown = False
16+
yield
17+
# Optionally reset after test as well for cleanup
18+
EmbeddingsCache._warning_shown = False
19+
20+
1221
@pytest.mark.asyncio
1322
async def test_sync_methods_warn_with_async_only_client(async_client, caplog):
1423
"""Test that sync methods warn when only async client is provided."""
15-
# Reset the warning flag for testing
16-
EmbeddingsCache._warning_shown = False
17-
1824
# Initialize EmbeddingsCache with only async_redis_client
1925
cache = EmbeddingsCache(name="test_cache", async_redis_client=async_client)
2026

@@ -49,9 +55,6 @@ async def test_sync_methods_warn_with_async_only_client(async_client, caplog):
4955

5056
def test_no_warning_with_sync_client(redis_url):
5157
"""Test that no warning is shown when sync client is provided."""
52-
# Reset the warning flag for testing
53-
EmbeddingsCache._warning_shown = False
54-
5558
# Create sync redis client from redis_url
5659
sync_client = Redis.from_url(redis_url)
5760

@@ -73,9 +76,6 @@ def test_no_warning_with_sync_client(redis_url):
7376
@pytest.mark.asyncio
7477
async def test_async_methods_no_warning(async_client):
7578
"""Test that async methods don't trigger warnings."""
76-
# Reset the warning flag for testing
77-
EmbeddingsCache._warning_shown = False
78-
7979
# Initialize EmbeddingsCache with only async_redis_client
8080
cache = EmbeddingsCache(name="test_cache", async_redis_client=async_client)
8181

0 commit comments

Comments
 (0)