feat: adding new operations to ValkeyDocumentStore#2910
feat: adding new operations to ValkeyDocumentStore#2910davidsbatista wants to merge 4 commits intomainfrom
ValkeyDocumentStore#2910Conversation
anakin87
left a comment
There was a problem hiding this comment.
Good work overall.
I left some comments and questions.
| try: | ||
| docs_to_update = self.filter_documents(filters) | ||
| for doc in docs_to_update: | ||
| doc.meta = dict(doc.meta or {}) |
There was a problem hiding this comment.
this triggers a warning due to deepset-ai/haystack#10650
let's find a way to avoid mutating the attribute in-place
Warning: Mutating attribute 'meta' on an instance of 'Document' can lead to unexpected behavior by affecting other parts of the pipeline that use the same dataclass instance. Use dataclasses.replace(instance, meta=new_value) instead. See https://docs.haystack.deepset.ai/docs/custom-components#requirements for details.
| try: | ||
| docs_to_update = await self.filter_documents_async(filters) | ||
| for doc in docs_to_update: | ||
| doc.meta = dict(doc.meta or {}) |
There was a problem hiding this comment.
same issue here (in-place mutation)
integrations/valkey/src/haystack_integrations/document_stores/valkey/document_store.py
Show resolved
Hide resolved
integrations/valkey/src/haystack_integrations/document_stores/valkey/document_store.py
Show resolved
Hide resolved
| @staticmethod | ||
| def _get_doc_meta_value(doc: Document, field_name: str) -> Any: | ||
| """Get metadata value from a document; field_name can be 'category', 'meta.category', or 'meta_category'.""" | ||
| key = ValkeyDocumentStore._normalize_metadata_field_name(field_name) | ||
| meta_key = key[5:] if key.startswith("meta_") else key # key for doc.meta | ||
| return (doc.meta or {}).get(meta_key) |
There was a problem hiding this comment.
since we already normalize fields names, is this method needed?
| counts: dict[str, int] = {} | ||
| for field_name in metadata_fields: | ||
| meta_key = ValkeyDocumentStore._normalize_metadata_field_name(field_name) | ||
| user_name = meta_key[5:] if meta_key.startswith("meta_") else meta_key |
There was a problem hiding this comment.
nit: user_name seems a bit misleading variable name
| assert doc.meta["score"] >= 0.7 | ||
|
|
||
| # --- delete_by_filter, update_by_filter, count_documents_by_filter --- | ||
|
|
There was a problem hiding this comment.
Here in tests, could we use the tests you added in haystack/testing/document_store.py?
… in-place doc modifications
Related Issues
Proposed Changes:
Adding the following new operations, both sync and async:
How did you test it?
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.