Rename index #1170
Rename index #1170
Conversation
WalkthroughIndex.update() gained an optional new_uid parameter to support renaming; payload now includes "uid" when renaming and requires at least one of primary_key or new_uid. swap_indexes docstring and docs formatting were tweaked. Tests added for rename and swap behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Index
participant API
Caller->>Index: update(primary_key?, new_uid?)
rect rgb(245,250,240)
Note over Index: Build payload conditionally
Index->>Index: payload = {}
alt primary_key provided
Index->>Index: payload["primaryKey"] = primary_key
end
alt new_uid provided
Index->>Index: payload["uid"] = new_uid
end
alt neither provided
Index-->>Caller: raise ValueError
end
end
Index->>API: PATCH /indexes/{uid} with payload
API-->>Index: TaskInfo
Index-->>Caller: TaskInfo
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)meilisearch/index.py (1)
🪛 Ruff (0.14.5)meilisearch/index.py138-140: Avoid specifying long messages outside the exception class (TRY003) 🔇 Additional comments (4)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
meilisearch/client.py
Outdated
| indexes: | ||
| List of indexes to swap (ex: [{"indexes": ["indexA", "indexB"]}). | ||
| List of indexes to swap ex: | ||
| [{"indexes": ["indexA", "indexB"]}) # default rename to false |
There was a problem hiding this comment.
not valid json
maybe
[{"indexes": ["indexA", "indexB"]},
{"indexes": ["indexA", "indexB"], "rename": false},
{"indexes": ["indexA", "indexB"], "rename": true}]
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/index/test_index.py (1)
255-255: Clean up redundant comment marker.Line 255 has an extra
#creating a redundant comment marker.Apply this diff:
- # # Verify the old UID no longer exists + # Verify the old UID no longer exists
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/conf.py(1 hunks)meilisearch/client.py(1 hunks)meilisearch/index.py(2 hunks)tests/client/test_client_swap_meilisearch.py(2 hunks)tests/index/test_index.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/index/test_index.py (5)
tests/conftest.py (1)
client(15-16)meilisearch/client.py (2)
index(226-242)wait_for_task(639-666)meilisearch/index.py (4)
update(107-137)wait_for_task(241-268)fetch_info(139-151)get_primary_key(153-161)meilisearch/task.py (1)
wait_for_task(174-212)meilisearch/errors.py (1)
MeilisearchApiError(28-51)
tests/client/test_client_swap_meilisearch.py (3)
tests/conftest.py (2)
client(15-16)empty_index(109-117)meilisearch/client.py (3)
index(226-242)swap_indexes(534-556)wait_for_task(639-666)meilisearch/index.py (2)
wait_for_task(241-268)fetch_info(139-151)
⏰ 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: Seer Code Review
🔇 Additional comments (7)
docs/conf.py (1)
61-65: LGTM! Minor formatting improvements.The quote style standardization and spacing adjustments improve code consistency without affecting functionality.
meilisearch/client.py (1)
540-543: LGTM! Clear documentation examples.The expanded examples effectively illustrate the three usage patterns for the
renameparameter in swap operations.tests/index/test_index.py (2)
239-257: LGTM! Well-structured rename test.The test properly validates:
- Successful rename operation via
update(new_uid=...)- New UID is accessible with correct info
- Old UID becomes inaccessible (raises MeilisearchApiError)
260-275: LGTM! Excellent test for combined update.This test validates the important use case of updating both primary key and UID in a single operation, ensuring atomicity and correct behavior.
meilisearch/index.py (1)
107-137: LGTM! Clean and flexible implementation.The conditional payload construction is well-designed, allowing:
- Primary key update only
- Rename only
- Combined update and rename
- Handles optional parameters correctly
The implementation properly avoids sending unnecessary fields in the payload.
tests/client/test_client_swap_meilisearch.py (2)
49-60: LGTM! Validates error handling with rename=False.This test correctly verifies that when
rename=False(or default), swapping with a non-existent index produces anindex_not_founderror as expected.
76-91: LGTM! Validates successful rename via swap.This test effectively validates that
rename=Trueenables renaming an index by "swapping" to a non-existent target name, and confirms the renamed index is accessible.
There was a problem hiding this comment.
Actionable comments posted: 1
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)
107-127: Update docstring to reflect rename capability.The docstring on Line 108 states "Update the index primary-key" but the method now also supports renaming via the
new_uidparameter. Update the summary to reflect both capabilities.Apply this diff:
- def update(self, primary_key: Optional[str] = None, new_uid: Optional[str] = None) -> TaskInfo: - """Update the index primary-key. + def update(self, primary_key: Optional[str] = None, new_uid: Optional[str] = None) -> TaskInfo: + """Update the index primary key or rename the index.
🧹 Nitpick comments (2)
tests/index/test_index.py (1)
278-285: Consider stricter assertion.The assertion on Line 285 uses
or, which passes if either string appears in the error message. Consider usingandto ensure both parameter names are mentioned, providing a more robust validation of the error message.Apply this diff:
- assert "primary_key" in str(exc.value) or "new_uid" in str(exc.value) + assert "primary_key" in str(exc.value) and "new_uid" in str(exc.value)meilisearch/index.py (1)
128-131: Validation correctly prevents empty payload.The validation logic properly addresses the concern from the previous review about allowing an empty payload, ensuring at least one parameter is provided before making the API call.
Optional: The static analysis tool suggests avoiding long error messages inline (TRY003), but this is a style preference and not a functional issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
meilisearch/index.py(2 hunks)tests/index/test_index.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
meilisearch/index.py (1)
meilisearch/models/task.py (1)
TaskInfo(77-106)
tests/index/test_index.py (5)
tests/conftest.py (1)
client(15-16)meilisearch/client.py (2)
index(226-242)wait_for_task(639-666)meilisearch/index.py (4)
update(107-142)wait_for_task(246-273)fetch_info(144-156)get_primary_key(158-166)meilisearch/task.py (1)
wait_for_task(174-212)meilisearch/errors.py (1)
MeilisearchApiError(28-51)
🪛 Ruff (0.14.5)
meilisearch/index.py
129-131: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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: integration-tests (3.12)
🔇 Additional comments (2)
tests/index/test_index.py (2)
239-258: LGTM! Thorough rename test.The test correctly verifies the rename functionality and properly creates a new index reference after renaming rather than reusing the original Index object, which becomes stale after the UID changes.
260-276: LGTM! Combined update test is well-structured.The test properly validates that both
primary_keyandnew_uidcan be updated in a single call, and correctly verifies both changes took effect.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Strift
left a comment
Strift
left a comment
There was a problem hiding this comment.
Thanks for this contribution 🙌
3 participants
This pull request adds complete support for renaming Meilisearch indexes using both the Index.update() method and Client.swap_indexes().
Key Changes
Index RenamingviaIndex.update()Added a new optional parameter new_uid to the update() method. This allows renaming an index using the standard index update endpoint.Example payloadSupport via Client.swap_indexes()
No code changes required.
The rename operation can be passed directly in the swap payload using
rename:True or FalseAdded Tests
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.