Add sharding support for Network methods #709
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #709 +/- ##
==========================================
- Coverage 85.89% 85.84% -0.06%
==========================================
Files 19 19
Lines 5950 6054 +104
==========================================
+ Hits 5111 5197 +86
- Misses 839 857 +18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WalkthroughAdds a public Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Caller
participant C as Client
participant H as HttpClient
participant S as Meilisearch Server
rect rgb(240,245,255)
note over Dev,C: Read network state
Dev->>C: get_network_state()
C->>H: GET /network
H->>S: GET /network
S-->>H: 200 NetworkState
H-->>C: NetworkState
C-->>Dev: NetworkState
end
rect rgb(240,255,240)
note over Dev,C: Update sharding / self remote
Dev->>C: set_sharding(enabled) / set_self_remote(name)
C->>C: Build NetworkUpdate
C->>H: PATCH /network (NetworkUpdate)
H->>S: PATCH /network
alt success
S-->>H: 200 NetworkState
H-->>C: NetworkState
C-->>Dev: NetworkState
else error
S-->>H: 4xx/5xx Error
H-->>C: Error
C-->>Dev: Error
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/tasks.rs (2)
773-798: New test: processing task withremotes— LGTM; consider also coveringSucceededTask.Add a small test case for the succeeded variant to fully cover the three task kinds.
800-803: Redundantuse super::*;importThis second import is unnecessary. Remove to reduce noise.
- use super::*;src/client.rs (2)
1251-1293: Test for network update and remotes deserialization — LGTMGood assertion on
writeApiKey. Consider adding a test forset_self_remote()and one for GET /network.
1152-1190: Network endpoints: GET/PATCH /network — PATCH returns 200 OK (verified)
Docs for Meilisearch v1.19 confirm PATCH /network returns 200 OK; no change needed to the response code.
- Optional: add a small helper to update remotes (e.g., set_remotes(...)) and a unit test for set_self_remote.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/client.rs(3 hunks)src/lib.rs(1 hunks)src/network.rs(1 hunks)src/search.rs(2 hunks)src/tasks.rs(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-12T13:28:23.700Z
Learnt from: LukasKalbertodt
PR: meilisearch/meilisearch-rust#625
File: src/search.rs:368-370
Timestamp: 2025-06-12T13:28:23.700Z
Learning: In the Meilisearch Rust client, `SearchQuery` serializes its per-query federation settings under the key `federationOptions`; only the top-level multi-search parameter is named `federation`.
Applied to files:
src/search.rs
🧬 Code graph analysis (2)
src/client.rs (2)
src/request.rs (1)
body(49-56)src/features.rs (1)
update(102-114)
src/search.rs (1)
src/client.rs (2)
None(1283-1283)None(1362-1362)
⏰ 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
🔇 Additional comments (9)
src/lib.rs (1)
247-249: Exposenetworkmodule publicly — LGTMMakes the new network API available at meilisearch_sdk::network.
src/search.rs (2)
769-775: Initializeremote: None— LGTMKeeps previous behavior while allowing future remote selection.
418-424: Confirmed: per-queryremotebelongs underfederationOptions(resolved)Confirmed — Meilisearch v1.19 expects a per-query string at queries[].federationOptions.remote. Example:
{ "federation": {}, "queries": [ { "indexUid":"movies","q":"batman","federationOptions":{"remote":"ms-00"} }, { "indexUid":"movies","q":"batman","federationOptions":{"remote":"ms-01"} } ] }Optional: add a convenience helper (e.g., with_search_query_and_remote) to mirror with_search_query_and_weight.
src/tasks.rs (5)
2-2: Import forserde_json::{Map, Value}— LGTMRequired for flexible
remotespayloads.
161-164: AddremotestoSucceededTask— LGTMOptional and skipped when absent; compatible with 1.19 responses.
181-184: AddremotestoEnqueuedTask— LGTMMatches server behavior for enqueued tasks.
203-206: AddremotestoProcessingTask— LGTMMatches server behavior for processing tasks.
753-771: New test: enqueued task withremotes— LGTMCovers the new field.
src/client.rs (1)
11-12: Import network types — LGTMWires client to the new network module.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/tasks.rs (4)
2-2: Import looks good; consider a shared type alias for reuse.Define a crate-wide alias to keep the shape consistent across modules (tasks/network) and ease future refactors:
For example:
pub type Remotes = serde_json::Map<String, serde_json::Value>;Then use
Option<Remotes>for these fields.
161-164: Add remotes field: OK; verify potential secret exposure in Debug output.
- If task remotes can include secrets (e.g., writeApiKey),
#[derive(Debug)]will print them. Consider redaction (custom Debug/newtype) or documenting that these structs may contain sensitive data.- Also, add a test for Succeeded (and possibly Failed) variants containing remotes for parity.
749-770: Good coverage for enqueued; add a stronger assertion.Also assert nested value equality to ensure structure is preserved, and consider adding a Succeeded/Failed counterpart test.
Example:
let ms00 = remotes.get("ms-00").unwrap(); assert_eq!(ms00.get("status").and_then(|v| v.as_str()), Some("ok"));
772-797: Processing test looks good; mirror stronger assertion.Add a direct check on the nested "status" value, similar to the enqueued test suggestion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/tasks.rs(6 hunks)
⏰ 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
🔇 Additional comments (3)
src/tasks.rs (3)
181-184: Same concerns as in SucceededTask.Duplicate of earlier comment: verify secret exposure via Debug; consider redaction and add a Succeeded/Failed test for remotes.
203-206: Same concerns as in SucceededTask.Duplicate of earlier comment: verify secret exposure via Debug; consider redaction and add a Succeeded/Failed test for remotes.
843-843: Pattern updated with..to tolerate new fields — LGTM.Prevents fragile test matches after adding
remotes.
|
@curquiza could you take a look when you get the time. |
|
Build succeeded: |
2 participants
Pull Request
This PR aims to bring the SDK close to the Mealisearch Meilisearch 1.19 features and add support for sharding .
Related issue
Fixes #700
What does this PR do?
As per the tasks described in the origina issue #700
Update the Network methods to accept sending the sharding parameter
Update the Network methods to include
remotes.[remoteName].writeApiKeyin the responsesUpdate the Tasks methods to include remotes objects in the tasks reponse index update method to allow renaming
Add new test cases to test implementation
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
New Features
Improvements
Tests