Skip to content

add shards field to NetworkState#774

Open
valkrypton wants to merge 1 commit intomeilisearch:mainfrom
valkrypton:network-api-changes
Open

add shards field to NetworkState#774
valkrypton wants to merge 1 commit intomeilisearch:mainfrom
valkrypton:network-api-changes

Conversation

@valkrypton
Copy link

@valkrypton valkrypton commented Mar 6, 2026

Pull Request

Related issue

Fixes #<issue_number>

What does this PR do?

  • Adds newly introduced shards to NetworkState in meilisearch 1.37.0.

PR checklist

Please check if your PR fulfills the following requirements:

  • Did you use any AI tool while implementing this PR (code, tests, docs, etc.)? If yes, disclose it in the PR description and describe what it was used for. AI usage is allowed when it is disclosed.
  • 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?

Thank you so much for contributing to Meilisearch!

Summary by CodeRabbit

  • New Features
    • Added support for shard configuration management, enabling configuration of remote nodes for network shards with the ability to add and remove remote configurations.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 6, 2026

📝 Walkthrough

Walkthrough

Adds shard management support to the network module by introducing a new ShardsConfig struct and two type aliases (ShardsMap, ShardsUpdateMap). Extends existing NetworkState and NetworkUpdate structs with optional shard-related fields to support remote management operations.

Changes

Cohort / File(s) Summary
Shard Configuration Types
src/network.rs
Introduces ShardsConfig struct with fields for managing remotes, plus ShardsMap and ShardsUpdateMap type aliases for mapping shard identifiers to configuration or updates.
Extended Network Structs
src/network.rs
Extends NetworkState with optional shards field and NetworkUpdate with optional shards field (serialization skipped when None) to integrate shard management into existing network structures.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Hops of joy through the network, so spry—
New shards take flight with remotes nearby!
Config structures bloom, camelCase neat,
A serde-blessed addition, delightfully sweet!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'add shards field to NetworkState' directly and accurately describes the main change in the PR, which adds a shards field to the NetworkState struct.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

🧹 Nitpick comments (1)
src/network.rs (1)

16-22: The ShardsConfig struct aligns with Meilisearch 1.37.0 API expectations and follows established codebase patterns. The three fields (remotes, addRemotes, removeRemotes) match the API shard object schema, and the struct is appropriately wrapped in Option via ShardsUpdateMap, making the Default derive unnecessary (consistent with how RemoteConfig is handled).

Note: The Meilisearch API allows partial updates with any combination of the three shard fields; the current design requires all three to be present or the entire shard to be None. If more granular field-level optionality is needed, consider using Option<Vec<String>> for individual fields with #[serde(skip_serializing_if = "Option::is_none")].

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/network.rs` around lines 16 - 22, ShardsConfig should not derive Default
(keep only Clone, Serialize, Deserialize) to match RemoteConfig and the rest of
the codebase; remove any Default derive from the ShardsConfig declaration
(symbol: ShardsConfig) so the struct remains wrapped in Option via
ShardsUpdateMap and supports partial-shard omission, and if you need more
granular partial updates change the three fields remotes, add_remotes,
remove_remotes to Option<Vec<String>> and add #[serde(skip_serializing_if =
"Option::is_none")] to each field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/network.rs`:
- Around line 16-22: ShardsConfig should not derive Default (keep only Clone,
Serialize, Deserialize) to match RemoteConfig and the rest of the codebase;
remove any Default derive from the ShardsConfig declaration (symbol:
ShardsConfig) so the struct remains wrapped in Option via ShardsUpdateMap and
supports partial-shard omission, and if you need more granular partial updates
change the three fields remotes, add_remotes, remove_remotes to
Option<Vec<String>> and add #[serde(skip_serializing_if = "Option::is_none")] to
each field.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 40abfbd4-7185-42ca-a39c-9b1ddc103d8c

📥 Commits

Reviewing files that changed from the base of the PR and between 82fe194 and 2b9dbd1.

📒 Files selected for processing (1)
  • src/network.rs

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant