You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix: add delete_by_filter_async to InMemoryDocumentStore - #13356
InMemoryDocumentStore exposes an async counterpart for every public method (update_by_filter_async, count_documents_by_filter_async, delete_all_documents_async, ...), except delete_by_filter. The shared DeleteByFilterAsyncTest mixin in haystack/testing/document_store_async.py already exists, but it was not mixed into TestMemoryDocumentStore, so the gap went unnoticed.
This PR:
adds InMemoryDocumentStore.delete_by_filter_async(filters) -> int, which runs the existing sync delete_by_filter in the store's thread-pool executor, following the same pattern as update_by_filter_async;
adds DeleteByFilterAsyncTest to TestMemoryDocumentStore;
adds a release note.
How did you test it?
Added DeleteByFilterAsyncTest first and confirmed the new tests failed with AttributeError: 'InMemoryDocumentStore' object has no attribute 'delete_by_filter_async'.
After the implementation: pytest test/document_stores/test_in_memory.py → 384 passed, 8 skipped (the new tests run for both the shared and non-shared store test classes).
ruff check ., ruff format --check ., codespell, reno lint, and mypy on the changed modules pass.
Notes for the reviewer
The change is purely additive and delegates to the existing sync delete_by_filter, so filter validation and ValueError behavior are identical.
AI assistance was used to prepare this change; I reviewed and tested it.
I have added unit tests and updated the docstrings.
I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
Hi @kerthans, thanks a lot for your contribution! 🙏
We noticed that the Contributor License Agreement (CLA) check (license/cla) hasn't passed yet, so we've temporarily moved this PR to draft and paused the review assignment.
To get your PR reviewed, please sign the CLA via the link in the license/cla check below (or in the CLA bot comment). As soon as the check turns green, this PR will automatically be marked ready for review again and a reviewer will be re-assigned.
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You have signed the CLA already but the status is still pending? Let us recheck it.
Thanks for signing the CLA, @kerthans! 🎉 This PR is now ready for review again and the reviewer has been re-assigned.
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issues
DeleteAllAsyncTest,DeleteByFilterAsyncTest, #10952, which addeddelete_all_documents_asyncbut notdelete_by_filter_async.Proposed Changes:
InMemoryDocumentStoreexposes an async counterpart for every public method (update_by_filter_async,count_documents_by_filter_async,delete_all_documents_async, ...), exceptdelete_by_filter. The sharedDeleteByFilterAsyncTestmixin inhaystack/testing/document_store_async.pyalready exists, but it was not mixed intoTestMemoryDocumentStore, so the gap went unnoticed.This PR:
InMemoryDocumentStore.delete_by_filter_async(filters) -> int, which runs the existing syncdelete_by_filterin the store's thread-pool executor, following the same pattern asupdate_by_filter_async;DeleteByFilterAsyncTesttoTestMemoryDocumentStore;How did you test it?
DeleteByFilterAsyncTestfirst and confirmed the new tests failed withAttributeError: 'InMemoryDocumentStore' object has no attribute 'delete_by_filter_async'.pytest test/document_stores/test_in_memory.py→ 384 passed, 8 skipped (the new tests run for both the shared and non-shared store test classes).ruff check .,ruff format --check .,codespell,reno lint, andmypyon the changed modules pass.Notes for the reviewer
The change is purely additive and delegates to the existing sync
delete_by_filter, so filter validation andValueErrorbehavior are identical.AI assistance was used to prepare this change; I reviewed and tested it.
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.