Skip to content

Conversation

@jawad-khan
Copy link

@jawad-khan jawad-khan commented Nov 14, 2025

Pull Request

This PR introduces sharding support to the Meilisearch Python SDK. I have tested the sharding functionality locally using two remote nodes, and all related test cases passed successfully.

Related issue

Fixes #<1137>

What does this PR do?

As mentioned in the issue, I have tested all of these points locally and added tests for the parts that can be covered within the test suite.

  • Update the Network methods to accept sending the sharding parameter
  • Update the Network methods to include remotes.[remoteName].writeApiKey in the responses
  • Update the Tasks methods to include remotes objects in the tasks response index update method to allow renaming
  • Add new test cases to test implementation

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Summary by CodeRabbit

  • New Features

    • Network federation with optional sharding across remote instances.
    • Configurable API keys for secure communication between federated nodes.
    • Tasks now carry optional network metadata.
  • Tests

    • Added tests covering sharding configuration, multi-instance federation, and cleanup helpers.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 14, 2025

Walkthrough

Adds an optional network: Optional[Dict[str, Any]] = None field to the Task model and updates/introduces tests and a test helper to exercise network sharding and remote write API key plumbing.

Changes

Cohort / File(s) Summary
Model Extension
meilisearch/models/task.py
Added public field network: Optional[Dict[str, Any]] = None to the Task model.
Network tests updated
tests/client/test_client_network.py, tests/client/test_client_multi_search_meilisearch.py
Added top-level "sharding": True to network payloads, included writeApiKey entries for remotes, added assertions checking response["sharding"], and call disable_sharding(client) during cleanup.
New sharding test
tests/client/test_client_sharding.py
Added pytest file that sets network options (self, one remote with url, searchApiKey, writeApiKey, and sharding: True), calls add_or_update_networks, verifies get_all_networks() matches expectations, and disables sharding after.
Test helper
tests/test_utils.py
Added def disable_sharding(client) which calls client.add_or_update_networks(body={"sharding": False}) to revert sharding state.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Areas to review closely:
    • Ensure Task.network is correctly included/excluded in Task serialization/deserialization and does not break schema consumers.
    • Verify new writeApiKey fields in tests match API expectations and any validation the server enforces.
    • Check disable_sharding is safe to call in test teardown and does not require additional parameters or error handling.
    • Confirm test fixtures referenced (e.g., enable_network_options, REMOTE_MS_*, BASE_URL) are present and compatible with the updated payload shapes.

Poem

🐰 I hopped through code with a jittery cheer,
A network field added, now tasks hold it near,
Remotes got keys and sharding set true,
Tests tidy it up with a hop and a brew,
🥕 onward we go, with a soft debug cheer.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Add sharding support' is vague and generic, lacking specificity about what sharding support entails or which components are affected. Consider a more descriptive title such as 'Add sharding support to Network methods and Task model' to better convey the scope and nature of the changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
tests/client/test_client_multi_search_meilisearch.py (1)

107-107: Add assertion to verify sharding was configured.

The response from add_or_update_networks should be validated to confirm that sharding was successfully enabled, similar to the assertion in test_client_network.py line 37.

Apply this diff to add the missing assertion:

     assert "results" not in resp
+    assert resp["sharding"] is True
     assert "results" not in response
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a54e3c2 and d43103a.

📒 Files selected for processing (4)
  • meilisearch/models/task.py (1 hunks)
  • tests/client/test_client_multi_search_meilisearch.py (1 hunks)
  • tests/client/test_client_network.py (1 hunks)
  • tests/client/test_client_sharding.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/client/test_client_network.py (2)
tests/conftest.py (1)
  • client (15-16)
meilisearch/client.py (1)
  • add_or_update_networks (779-799)
tests/client/test_client_sharding.py (2)
tests/conftest.py (1)
  • client (15-16)
meilisearch/client.py (2)
  • add_or_update_networks (779-799)
  • get_all_networks (801-814)
🔇 Additional comments (4)
tests/client/test_client_multi_search_meilisearch.py (1)

84-101: LGTM! Network configuration correctly includes sharding and writeApiKey fields.

The addition of "sharding": True and "writeApiKey" fields to the network configuration aligns with the PR's objective of introducing sharding support.

tests/client/test_client_network.py (1)

17-37: LGTM! Test properly validates sharding configuration.

The test correctly adds the sharding flag and writeApiKey fields to the network configuration, and includes appropriate assertions to verify the response.

tests/client/test_client_sharding.py (1)

1-35: LGTM! Comprehensive test coverage for sharding functionality.

This new test file provides thorough validation of the sharding feature, including:

  • Setting network configuration with sharding enabled
  • Validating writeApiKey fields are properly stored and retrieved
  • Confirming the sharding flag persists across update/get operations
meilisearch/models/task.py (1)

24-24: The Task.network field is correctly implemented and will deserialize from API responses.

The field definition follows the correct pattern for optional generic dictionaries. When the network experimental feature is enabled, the field will contain replication metadata (origin or remotes data), and Pydantic will automatically deserialize it from the API response. The existing network tests in the codebase validate network configuration endpoints; the field itself will be populated transparently when tasks are retrieved.

@Strift Strift added the enhancement New feature or request label Nov 17, 2025
@Strift Strift linked an issue Nov 17, 2025 that may be closed by this pull request
4 tasks
@jawad-khan jawad-khan force-pushed the jawad/sharding-support branch from d43103a to 48b1dd0 Compare November 17, 2025 12:31
@curquiza
Copy link
Member

@jawad-khan thanks for your PR
can you fix the tests before we review?

@curquiza curquiza changed the title feat: Added sharding support for python sdk Add sharding support Nov 21, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
tests/test_utils.py (1)

36-37: Helper implementation looks correct; consider minor clarity improvements

The helper cleanly delegates to client.add_or_update_networks with a minimal payload and is appropriate for test reuse. To make its intent clearer to future readers, you might optionally add a short docstring and a return type hint (-> None), since this will likely be shared across multiple test modules.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d43103a and c25fc71.

📒 Files selected for processing (5)
  • meilisearch/models/task.py (1 hunks)
  • tests/client/test_client_multi_search_meilisearch.py (3 hunks)
  • tests/client/test_client_network.py (2 hunks)
  • tests/client/test_client_sharding.py (1 hunks)
  • tests/test_utils.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • meilisearch/models/task.py
  • tests/client/test_client_multi_search_meilisearch.py
  • tests/client/test_client_network.py
  • tests/client/test_client_sharding.py
🧰 Additional context used
🧬 Code graph analysis (1)
tests/test_utils.py (2)
tests/conftest.py (1)
  • client (15-16)
meilisearch/client.py (1)
  • add_or_update_networks (779-799)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[v1.19.0] Add sharding support

3 participants