Fix LangCache clear() 400 error by using flush() API #428
Conversation
… empty attributes - Changed delete() and adelete() to raise NotImplementedError - Added validation to delete_by_attributes() and adelete_by_attributes() to reject empty attributes - Updated all related tests to verify new behavior - Added new tests for delete_by_attributes with both valid and empty attributes Co-authored-by: abrookins <[email protected]>
|
@copilot The API deletes all entries unless you give it attributes to filter on. |
…pty attributes - Changed delete() to use self._client.flush() instead of raising NotImplementedError - Changed adelete() to use self._client.flush_async() - Updated error messages in delete_by_attributes to suggest using delete()/clear() - Updated all tests to verify flush() is called - This fixes the 400 error by using the proper API endpoint for clearing all entries Co-authored-by: abrookins <[email protected]>
You're right! I found that the LangCache SDK provides a |
|
@copilot No need to raise on an empty dict; just ignore it. |
- Changed delete_by_attributes() to return {} when attributes is empty
- Changed adelete_by_attributes() to return {} when attributes is empty
- Updated tests to verify empty attributes are ignored without calling API
- This prevents accidentally calling API with empty dict which would cause 400 error
Co-authored-by: abrookins <[email protected]>
|
@copilot Those client methods return object of this class: class DeleteQueryResponse(BaseModel):
r"""Response indicating the result of a cache entries deletion operation"""
deleted_entries_count: Annotated[int, pydantic.Field(alias="deletedEntriesCount")]
r"""The number of cache entries successfully deleted."""So technically, an empty dict return value is incorrect; use a dict with the correct shape: |
- Changed delete_by_attributes() to return {"deleted_entries_count": 0} instead of {}
- Changed adelete_by_attributes() to return {"deleted_entries_count": 0} instead of {}
- Updated tests to verify correct response shape
- This matches the DeleteQueryResponse structure from LangCache SDK
Co-authored-by: abrookins <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a critical bug where calling LangCache.clear() resulted in a 400 Bad Request error. The fix replaces the problematic delete_query(attributes={}) call with the proper flush() API method, and adds defensive handling for empty attributes.
- Replaces
delete_query(attributes={})withflush()/flush_async()indelete()andadelete()methods - Adds early return for empty attributes in
delete_by_attributes()andadelete_by_attributes()to prevent API errors - Adds comprehensive test coverage for clear, delete, and delete_by_attributes operations
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| redisvl/extensions/cache/llm/langcache.py | Updates delete() and adelete() to use flush() API; adds empty attributes guard in delete_by_attributes() and adelete_by_attributes() |
| tests/unit/test_langcache_semantic_cache.py | Updates existing tests to verify flush() is called; adds new tests for clear(), aclear(), and empty attributes handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot The SDK returns |
- Updated test mocks to use "deleted_entries_count" instead of "deleted_count" - This matches the actual DeleteQueryResponse from LangCache SDK - All tests now consistently use the correct key name Co-authored-by: abrookins <[email protected]>
| if not attributes: | ||
| # No attributes provided, return result with zero deletions | ||
| return {"deleted_entries_count": 0} |
There was a problem hiding this comment.
Should we add a warning here? Would it be helpful to the end user?
There was a problem hiding this comment.
Actually, thanks for asking about this. I've reconsidered and think raising an exception is best instead!
bsbodden
left a comment
bsbodden
left a comment
There was a problem hiding this comment.
Look good, I will implement in the Java version and confirm end-to-end functionality
|
@copilot In adelete_by_attributes and delete_by_attributes, raise an exception if attributes is an empty dict, something like ValueError("Cannot delete by attributes with an empty attributes dictionary.") Update tests, docstrings, etc. |
|
@copilot Try again: In adelete_by_attributes and delete_by_attributes, raise an exception if attributes is an empty dict, something like ValueError("Cannot delete by attributes with an empty attributes dictionary.") Update tests, docstrings, etc. |
- Changed delete_by_attributes() to raise ValueError instead of returning {"deleted_entries_count": 0}
- Changed adelete_by_attributes() to raise ValueError instead of returning {"deleted_entries_count": 0}
- Updated docstrings to indicate attributes cannot be empty and document the ValueError
- Updated tests to verify ValueError is raised with empty attributes
- Error message: "Cannot delete by attributes with an empty attributes dictionary."
Co-authored-by: abrookins <[email protected]>
Updated |
Adds integration with LangCache managed service for cloud-based semantic caching: - **LangCacheSemanticCache**: HTTP API client for LangCache service - Store/retrieve cached LLM responses via LangCache API - Semantic + exact search strategies - Distance threshold conversion (normalized/redis scales) - Metadata/attributes support for filtering - Delete operations (all, by ID, by attributes) - Unit tests with mocked HTTP client (13 tests passing) - **Demo Integration**: rag-multimodal demo uses LangCache - ServiceFactory configures LangCache from application.properties - RAGService integrates cache for LLM response caching - LangCacheIntegrationTest verifies configuration and connectivity - **Documentation**: Updated llmcache.adoc with examples and usage Port of redis-vl-python PR #418 with matching naming convention (LangCacheSemanticCache vs LangCacheWrapper to match SemanticCache pattern). Fixes default server URL to match Python implementation: https://aws-us-east-1.langcache.redis.io fix(langcache): Fix clear() 400 error with empty attributes Fixes the LangCacheSemanticCache.clear() method which was throwing a 400 Bad Request error with "attributes: cannot be blank." This fix ports the changes from redis-vl-python PR #428: redis/redis-vl-python#428 Changes: - Added flush() method that calls the dedicated /v1/caches/{id}/flush endpoint - Updated delete() to call flush() instead of deleteByAttributes(emptyMap()) - Modified deleteByAttributes() to return early with {"deleted_entries_count": 0} when attributes are null or empty, avoiding the 400 error from the API Tests added: - testFlush(): Verifies flush() calls the correct endpoint - testDelete(): Verifies delete() calls flush() - testClear(): Verifies clear() calls flush() via delete() - testDeleteByAttributesWithEmptyAttributes(): Verifies early return for empty attributes All 565 tests pass. Fixes issue where calling cache.clear() or cache.delete() would fail with: "400 Bad Request: attributes: cannot be blank" test(langcache): add integration tests for clear() fix Adds integration tests to verify the fix for the LangCache clear() 404 error. Tests added: - testDeleteDoesNotThrow400Error: Verifies delete() works without errors - testClearDoesNotThrow400Error: Verifies clear() works without errors - testFlushDeletesAllEntries: Verifies the new flush() method works - testDeleteByAttributesWithEmptyMapReturnsEarly: Verifies early return for empty attributes - testDeleteByAttributesWithNullReturnsEarly: Verifies early return for null attributes - testBasicStoreAndCheck: Sanity test for basic cache operations These tests confirmed the bug (404 error when calling delete with empty attributes) and verified the fix (using flush endpoint instead). Requires LANGCACHE_API_KEY environment variable to run. Configuration: - Updated core/build.gradle.kts to pass LANGCACHE_API_KEY to test tasks Related to redis-vl-python PR #428 chore: remove broken blog-demo module The blog-demo module was incomplete and causing build failures: - Missing HFTextVectorizer class - Type incompatibility issues with Filter classes - Missing dependencies Removed from settings.gradle.kts and deleted the directory. This cleans up the build and allows successful compilation. fix(test): fix LangCacheIntegrationTest for caches without attributes Fixed testBasicStoreAndCheck() to work with LangCache instances that don't have attributes configured: 1. Removed metadata from store() call (was causing 400 error) 2. Relaxed assertion on entryId (can be empty string in some API responses) All 6 LangCache integration tests now pass: - testDeleteDoesNotThrow400Error - testClearDoesNotThrow400Error - testFlushDeletesAllEntries - testDeleteByAttributesWithEmptyMapReturnsEarly - testDeleteByAttributesWithNullReturnsEarly - testBasicStoreAndCheck test(langcache): add integration tests for PR #429 parity Port integration tests from redis-vl-python PR #429: - Store/check with metadata and attribute filtering - Delete by ID, by attributes, and full flush - Error handling for caches without attributes configured - Comma handling in attribute values - Verify delete/clear alias behavior All 27 tests (16 unit + 11 integration) passing. Refs: redis-vl-python#429
Fix LangCache clear() 400 error by using flush() API
Summary
The LangCache API returns
400 Bad Request: "attributes: cannot be blank"whenclear()attempts to delete all entries by passingattributes={}todelete_query(). The fix uses the LangCache SDK's dedicatedflush()method to properly delete all entries.Changes
Core Fix
delete()andadelete()- Now callself._client.flush()andself._client.flush_async()respectively to delete all cache entries using the proper API endpoint.Additional Improvements
delete_by_attributes()andadelete_by_attributes()- Now raiseValueErrorif called with an empty attributes dictionary. This prevents misuse and provides a clear error message: "Cannot delete by attributes with an empty attributes dictionary." Users should usedelete()orclear()to delete all entries, or provide specific attributes to filter deletions.Tests
flush()is called fordelete()andclear()ValueErroris raised whendelete_by_attributes()is called with empty attributes"deleted_entries_count"key matching the SDKHow This Fixes The Issue
Original Problem:
Fixed Implementation:
Testing
✅ All 22 tests pass
✅ Tests verify
flush()is called for clearing all entries✅ Tests verify
ValueErroris raised for empty attributes with clear error message✅ Test mocks consistently use the correct SDK key names
✅ Manual verification confirms the fix resolves the original issue
Fixes Error when calling LangCacheSemanticCache.clear() method #427
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.