-
Notifications
You must be signed in to change notification settings - Fork 101
index renaming support #1191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
index renaming support #1191
Conversation
📝 WalkthroughWalkthroughThe changes add support for index renaming in the Meilisearch Python SDK. The swap_indexes method signature is updated to accept boolean values in mapping parameters (enabling a rename flag), the index update method's docstring is clarified to document UID renaming capability, and a code example demonstrates renaming an index via the update method with new_uid. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
meilisearch/index.py (1)
108-152: Docstring should document the primaryKey constraint.The implementation correctly:
- Validates that at least one of
primary_keyornew_uidis provided- Supports updating both fields in a single PATCH request
- Documents the stale instance behavior after renaming
However, the docstring omits a critical constraint:
primaryKeycan only be changed when the index is empty. Users callingupdate(primary_key="new_key")on an index with documents will receive an API error. Add documentation warning that the index must be empty before changing the primary key, similar to the existing warning about stale instances after rename.
🧹 Nitpick comments (1)
meilisearch/client.py (1)
652-674: Type signature correctly updated for rename support.The type change from
List[Mapping[str, List[str]]]toList[Mapping[str, List[str] | bool]]correctly models the new payload structure where each mapping can contain:
"indexes": a list of strings"rename": a boolean (optional)Minor nit: The docstring parameter description at line 657 says
indexes:but the actual parameter is namedparameters. Consider updating for consistency.📝 Suggested docstring fix
"""Swap two indexes. Parameters ---------- - indexes: + parameters: List of indexes to swap ex:
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.code-samples.meilisearch.yamlmeilisearch/client.pymeilisearch/index.py
🔇 Additional comments (1)
.code-samples.meilisearch.yaml (1)
817-818: LGTM!The code sample correctly demonstrates the index rename functionality using the
updatemethod withnew_uid, matching the PR objective to add therename_an_index_1example.
Pull Request
Related issue
Closes #1142
What does this PR do?
swap_indexesdefinition to accept the rename keyupdatefunction to indicate that it can rename indexes toorename_an_index_1key in.code-samples.meilisearch.ymlBesides this, the changes mentioned in #1142 were already done, including tests
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
Release Notes
Documentation
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.