feat: Added customMetadata in index apis #1179
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Index as Index API
participant HTTP as HTTP Layer
participant Server as Meilisearch Server
participant TaskModel as Task parser
rect rgb(240,248,255)
Client->>Index: call add_documents(..., metadata="Test metadata")
Note right of Index: metadata threaded\nthrough method stack
end
Index->>Index: _build_url(..., metadata="Test metadata")
Index->>HTTP: POST /indexes/{uid}/documents?customMetadata=Test%20metadata + body
HTTP->>Server: HTTP request
Server-->>HTTP: 202 Accepted (task JSON with customMetadata)
HTTP->>Index: response
Index->>TaskModel: parse task JSON (includes customMetadata)
TaskModel-->>Client: TaskInfo with customMetadata
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds support for the customMetadata field to index-related APIs in the Python SDK, aligning with recent Meilisearch server changes. The implementation adds an optional metadata parameter to all document manipulation methods (add, update, delete) that gets passed to the Meilisearch server as a query parameter.
Key changes:
- Added
customMetadatafield to theTaskmodel to capture metadata returned by the server - Updated all document methods in the
Indexclass to accept an optionalmetadataparameter - Enhanced test coverage by adding metadata assertions to document operation tests
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| meilisearch/models/task.py | Added customMetadata field as Optional[str] to Task model |
| meilisearch/index.py | Added metadata parameter to all document methods (add/update/delete variants) and updated URL building logic to include customMetadata as a query parameter |
| tests/index/test_index_document_meilisearch.py | Updated tests to pass metadata parameter and assert that customMetadata is persisted on task objects |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self, | ||
| documents: Sequence[Mapping[str, Any]], | ||
| batch_size: int = 1000, | ||
| primary_key: Optional[str] = None, |
There was a problem hiding this comment.
Inconsistent API design: In add_documents_in_batches, serializer is a keyword-only argument (line 521-522 uses *,), but in this method it's not keyword-only. For consistency, consider adding *, before serializer on line 906 to make it keyword-only, or update add_documents_in_batches to match this signature. The new metadata parameter should also be keyword-only to match the pattern in add_documents_in_batches.
| primary_key: Optional[str] = None, | |
| primary_key: Optional[str] = None, | |
| *, |
| index.wait_for_task(response.task_uid) | ||
| response = index.delete_documents(filter="genre=action", metadata="Test metadata") | ||
| task = index.wait_for_task(response.task_uid) | ||
| task.customMetadata = "Test metadata" |
There was a problem hiding this comment.
This line should be an assertion (assert task.customMetadata == "Test metadata") instead of an assignment. Unlike all other test cases in this file, this line assigns a value instead of verifying it, which means the test isn't actually validating that the metadata was properly persisted.
| task.customMetadata = "Test metadata" | |
| assert task.customMetadata == "Test metadata" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
meilisearch/index.py (4)
601-634: Inconsistent parameter style:metadatais positional here but keyword-only in similar methods.In
add_documents,add_documents_in_batches, andadd_documents_json, themetadataparameter is keyword-only (after*). However, inadd_documents_csv, it's a regular positional parameter.For API consistency, consider making
metadatakeyword-only:def add_documents_csv( self, str_documents: bytes, primary_key: Optional[str] = None, csv_delimiter: Optional[str] = None, + *, metadata: Optional[str] = None, ) -> TaskInfo:
636-666: Same inconsistency:metadatashould be keyword-only for consistency.def add_documents_ndjson( self, str_documents: bytes, primary_key: Optional[str] = None, + *, metadata: Optional[str] = None, ) -> TaskInfo:
751-781: Same inconsistency:metadatashould be keyword-only.def update_documents_ndjson( self, str_documents: str, primary_key: Optional[str] = None, + *, metadata: Optional[str] = None, ) -> TaskInfo:
820-853: Same inconsistency:metadatashould be keyword-only.def update_documents_csv( self, str_documents: str, primary_key: Optional[str] = None, csv_delimiter: Optional[str] = None, + *, metadata: Optional[str] = None, ) -> TaskInfo:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
meilisearch/index.py(37 hunks)meilisearch/models/task.py(1 hunks)tests/index/test_index_document_meilisearch.py(15 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/index/test_index_document_meilisearch.py (1)
meilisearch/index.py (14)
add_documents(479-514)update(108-152)add_documents_in_batches(516-562)add_documents_json(564-599)update_documents(714-749)update_documents_json(783-818)update_documents_in_batches(901-946)delete_document(948-975)delete_documents(978-1030)delete_all_documents(1032-1055)add_documents_csv(601-634)update_documents_csv(820-853)add_documents_ndjson(636-666)update_documents_ndjson(751-781)
meilisearch/index.py (2)
meilisearch/_httprequests.py (2)
put(120-140)post(98-108)meilisearch/models/task.py (1)
TaskInfo(79-108)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (31)
meilisearch/models/task.py (1)
25-25: LGTM -customMetadatafield added to Task model.The field correctly uses
Optional[str]and follows the existing CamelBase naming convention for API compatibility.tests/index/test_index_document_meilisearch.py (18)
31-40: LGTM - Test correctly verifies metadata propagation.
65-74: LGTM - Batch test correctly verifies metadata on each task.
112-120: LGTM - JSON serializer test with metadata verification.
129-140: LGTM - Raw documents test with metadata verification.
149-155: LGTM - Update documents test with metadata.
177-185: LGTM - Update documents JSON test with metadata.
194-205: LGTM - Update documents raw test with metadata.
393-398: LGTM - Update documents test with metadata.
423-432: LGTM - Update documents in batches test with metadata.
440-444: LGTM - Delete document test with metadata.
455-459: LGTM - Delete documents by ID test with metadata.
486-490: LGTM - Delete all documents test with metadata.
499-503: LGTM - Add documents CSV test with metadata.
525-530: LGTM - Update documents CSV test with metadata.
551-556: LGTM - Add documents JSON test with metadata.
563-567: LGTM - Update documents JSON test with metadata.
575-579: LGTM - Add documents NDJSON test with metadata.
587-592: LGTM - Update documents NDJSON test with metadata.meilisearch/index.py (12)
479-514: LGTM -add_documentsmethod correctly supports metadata parameter.The metadata is properly threaded through to
_build_urland documented in the docstring.
516-562: LGTM -add_documents_in_batchescorrectly propagates metadata to each batch.
564-599: LGTM -add_documents_jsoncorrectly propagates metadata.
668-712: LGTM -add_documents_rawcorrectly implements metadata as keyword-only.
714-749: LGTM -update_documentscorrectly implements metadata.
783-818: LGTM -update_documents_jsoncorrectly implements metadata as keyword-only.
855-899: LGTM -update_documents_rawcorrectly implements metadata as keyword-only.
901-946: LGTM -update_documents_in_batchescorrectly propagates metadata.Note: The parameter style follows the existing pattern for this method (positional optional parameters), though it differs from
add_documents_in_batcheswhich uses keyword-only parameters. This is a pre-existing inconsistency.
948-975: LGTM -delete_documentcorrectly implements metadata with URL encoding.
977-1030: LGTM -delete_documentscorrectly handles metadata in both the ids and filter branches.
1032-1055: LGTM -delete_all_documentscorrectly implements metadata.
2430-2445: LGTM -_build_urlcorrectly centralizes the metadata query parameter construction.Good approach to centralize the
metadata→customMetadatamapping in the URL builder.
|
@Strift Tests are failing because sharding require Enterprose addition now. Should we adjust test cases or somehow we can skip this check. |
|
@Strift Please re trigger test cases. |
jawad-khan
force-pushed
the
jawad/update-index-apis
branch
from
690ec39 to
f27c052
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
meilisearch/index.py (1)
901-908: Inconsistent API design:serializerandmetadatashould be keyword-only.Unlike
add_documents_in_batches(line 521-523), this method does not use*,to makeserializerandmetadatakeyword-only. This was flagged in a prior review.Apply this diff for consistency:
def update_documents_in_batches( self, documents: Sequence[Mapping[str, Any]], batch_size: int = 1000, primary_key: Optional[str] = None, + *, serializer: Optional[Type[JSONEncoder]] = None, metadata: Optional[str] = None, ) -> List[TaskInfo]:
🧹 Nitpick comments (4)
meilisearch/index.py (4)
601-634: Inconsistent API design:metadatashould be keyword-only.In
add_documents_csv,metadatais a positional parameter, but in similar methods likeadd_documentsandadd_documents_json,metadatais keyword-only (placed after*,). This inconsistency can lead to confusing API usage.Apply this diff for consistency:
def add_documents_csv( self, str_documents: bytes, primary_key: Optional[str] = None, csv_delimiter: Optional[str] = None, + *, metadata: Optional[str] = None, ) -> TaskInfo:
636-666: Inconsistent API design:metadatashould be keyword-only.Same issue as
add_documents_csv- for consistency with other methods,metadatashould be keyword-only.Apply this diff:
def add_documents_ndjson( self, str_documents: bytes, primary_key: Optional[str] = None, + *, metadata: Optional[str] = None, ) -> TaskInfo:
751-781: Inconsistent API design:metadatashould be keyword-only.For consistency with
update_documentsandupdate_documents_json, makemetadatakeyword-only.Apply this diff:
def update_documents_ndjson( self, str_documents: str, primary_key: Optional[str] = None, + *, metadata: Optional[str] = None, ) -> TaskInfo:
820-853: Inconsistent API design:metadatashould be keyword-only.For consistency, make
metadatakeyword-only.Apply this diff:
def update_documents_csv( self, str_documents: str, primary_key: Optional[str] = None, csv_delimiter: Optional[str] = None, + *, metadata: Optional[str] = None, ) -> TaskInfo:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
meilisearch/index.py(37 hunks)meilisearch/models/task.py(1 hunks)tests/index/test_index_document_meilisearch.py(15 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- meilisearch/models/task.py
🧰 Additional context used
🧬 Code graph analysis (1)
meilisearch/index.py (3)
meilisearch/_httprequests.py (3)
put(120-140)delete(142-147)post(98-108)meilisearch/models/task.py (1)
TaskInfo(79-108)meilisearch/client.py (1)
index(231-247)
🔇 Additional comments (30)
tests/index/test_index_document_meilisearch.py (19)
31-40: LGTM - metadata propagation test looks correct.The test properly verifies that the
customMetadataparameter is passed throughadd_documentsand returned on the completed task.
57-76: LGTM - batch metadata propagation test is thorough.The test correctly verifies that
customMetadatais propagated for each batch task inadd_documents_in_batches.
106-120: LGTM - JSON serializer with metadata test is correct.
123-140: LGTM - raw document addition with metadata test is correct.
143-155: LGTM - update documents with metadata test is correct.
171-185: LGTM - update documents JSON with metadata test is correct.
188-205: LGTM - update documents raw with metadata test is correct.
386-408: LGTM - update single document with metadata test is correct.
415-434: LGTM - update documents in batches with metadata test is correct.
437-447: LGTM - delete single document with metadata test is correct.
450-464: LGTM - delete documents by ID with metadata test is correct.
467-480: LGTM - delete documents with filter and metadata test is correct.The previously reported assignment bug on line 475 has been addressed.
483-493: LGTM - delete all documents with metadata test is correct.
496-506: LGTM - add documents CSV with metadata test is correct.
522-531: LGTM - update documents CSV with metadata test is correct.
548-557: LGTM - add documents JSON file with metadata test is correct.
560-569: LGTM - update documents JSON file with metadata test is correct.
572-581: LGTM - add documents NDJSON with metadata test is correct.
584-593: LGTM - update documents NDJSON with metadata test is correct.meilisearch/index.py (11)
948-975: LGTM - delete_document metadata implementation is correct.The inline URL construction with
parse.urlencodeproperly handles thecustomMetadataquery parameter.
1010-1030: LGTM - delete_documents metadata handling is correct.Both the deprecated
idspath and thefilterpath correctly append thecustomMetadataquery parameter when provided.
1032-1055: LGTM - delete_all_documents metadata implementation is correct.
2430-2445: LGTM -_build_urlmetadata handling is correct.The implementation properly adds
customMetadatato the query parameters only whenmetadatais provided, and usesparse.urlencodefor proper URL encoding.
479-514: LGTM -add_documentsmetadata implementation is correct.The
metadataparameter is properly defined as keyword-only and correctly passed to_build_url.
516-562: LGTM -add_documents_in_batchesmetadata propagation is correct.Metadata is properly propagated to each batch call.
564-599: LGTM -add_documents_jsonmetadata propagation is correct.
668-712: LGTM -add_documents_rawmetadata implementation is correct.
714-749: LGTM -update_documentsmetadata implementation is correct.
783-818: LGTM -update_documents_jsonmetadata propagation is correct.
855-899: LGTM -update_documents_rawmetadata implementation is correct.
jawad-khan
force-pushed
the
jawad/update-index-apis
branch
from
f27c052 to
7cbc800
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
meilisearch/index.py (1)
901-908: Inconsistent API design:serializershould be keyword-only.In
add_documents_in_batches(line 521-522),serializerandmetadataare keyword-only arguments (preceded by*,), but here they're positional. This inconsistency could confuse users.🔎 Proposed fix for consistent keyword-only arguments
def update_documents_in_batches( self, documents: Sequence[Mapping[str, Any]], batch_size: int = 1000, primary_key: Optional[str] = None, + *, serializer: Optional[Type[JSONEncoder]] = None, metadata: Optional[str] = None, ) -> List[TaskInfo]:
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
meilisearch/index.pymeilisearch/models/task.pytests/index/test_index_document_meilisearch.py
🚧 Files skipped from review as they are similar to previous changes (1)
- meilisearch/models/task.py
🧰 Additional context used
🧬 Code graph analysis (2)
tests/index/test_index_document_meilisearch.py (3)
meilisearch/index.py (14)
add_documents(479-514)update(108-152)wait_for_task(261-288)add_documents_in_batches(516-562)add_documents_json(564-599)update_documents(714-749)update_documents_json(783-818)update_documents_in_batches(901-946)delete_document(948-975)delete_documents(978-1030)delete_all_documents(1032-1055)add_documents_csv(601-634)update_documents_csv(820-853)update_documents_ndjson(751-781)meilisearch/models/task.py (1)
TaskInfo(79-108)meilisearch/task.py (1)
wait_for_task(174-212)
meilisearch/index.py (3)
meilisearch/_httprequests.py (3)
put(120-140)delete(142-147)post(98-108)meilisearch/models/task.py (1)
TaskInfo(79-108)meilisearch/client.py (1)
index(231-247)
🔇 Additional comments (21)
tests/index/test_index_document_meilisearch.py (14)
31-40: LGTM!Test correctly passes metadata and verifies it's returned in the task's
customMetadatafield.
65-74: LGTM!Test verifies metadata propagation for each batch task correctly.
112-120: LGTM!Correct metadata assertion for JSON document addition with custom serializer.
129-140: LGTM!Correct metadata assertion for raw document addition.
149-155: LGTM!Correct metadata assertion for document update with custom serializer.
177-185: LGTM!Correct metadata assertion for JSON document update.
194-205: LGTM!Correct metadata assertion for raw document update.
393-398: LGTM!Correct metadata assertion for document update.
423-432: LGTM!Correct metadata verification for batch document updates.
440-444: LGTM!Correct metadata assertion for single document deletion.
455-459: LGTM!Correct metadata assertion for batch document deletion by ID.
473-476: LGTM!The previously flagged assignment bug has been correctly fixed to use assertion (
==).
486-490: LGTM!Correct metadata assertion for delete all documents operation.
496-593: LGTM!All CSV, JSON, and NDJSON test variants correctly pass metadata and verify
customMetadatain task responses.meilisearch/index.py (7)
2430-2445: LGTM!The
_build_urlhelper correctly includescustomMetadatain query parameters when metadata is provided, using proper URL encoding.
479-562: LGTM!Metadata parameter correctly added to
add_documentsandadd_documents_in_batcheswith proper keyword-only enforcement and propagation to URL building.
564-712: LGTM!All
add_documents_*variants correctly accept and propagate the metadata parameter through to_build_url.
948-975: LGTM!The
delete_documentmethod correctly appendscustomMetadataas a query parameter when metadata is provided.
1015-1029: LGTM!Both the deprecated
idspath and thefilterpath correctly includecustomMetadatain the URL when metadata is provided.
1032-1055: LGTM!The
delete_all_documentsmethod correctly includescustomMetadatain the URL when provided.
751-899: LGTM!All
update_documents_*variants correctly accept and propagate the metadata parameter through to_build_url.
|
@Strift This is ready from my side. |
1 participant
Related issue
Fixes #1168
What does this PR do?
Update Index APIs to Support customMetadata
This PR updates the index-related APIs in the Python SDK to align with recent Meilisearch server changes and adds support for the new customMetadata field.
Key Updates
Added customMetadata support across index creation and update methods.
Synced request/response structures with the latest Meilisearch index API.
...
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.