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 <97182+abrookins@users.noreply.github.com>
|
@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 <97182+abrookins@users.noreply.github.com>
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 <97182+abrookins@users.noreply.github.com>
|
@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 <97182+abrookins@users.noreply.github.com>
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 <97182+abrookins@users.noreply.github.com>
| 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 <97182+abrookins@users.noreply.github.com>
Updated |
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.