feat: add sharding #705
feat: add sharding #705
Conversation
WalkthroughAdds Sharding and WriteAPIKey fields to public API types, updates JSON marshaling, extends integration and unit tests to cover new fields, and introduces TaskNetwork-related types for task responses. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Client Code
participant SDK as SDK Network Methods
participant API as Meilisearch Server
Dev->>SDK: UpdateNetwork({ sharding, remotes[...writeApiKey] })
SDK->>API: PUT /network (JSON with sharding, remotes.writeApiKey)
API-->>SDK: 200 OK (network with sharding, remotes.writeApiKey)
SDK-->>Dev: Network struct (Sharding, Remotes[*].WriteAPIKey)
note over SDK,API: Tests assert Sharding and WriteAPIKey round-trip
sequenceDiagram
autonumber
actor Dev as Client Code
participant SDK as SDK Task Methods
participant API as Meilisearch Server
Dev->>SDK: GetTask(...)
SDK->>API: GET /tasks/:uid
API-->>SDK: 200 OK (task.network { origin, remotes[...] })
SDK-->>Dev: Task struct with TaskNetwork (Origin, Remotes)
note over SDK: New TaskNetwork/Origin/TaskRemote types map response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNone found. Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #705 +/- ##
==========================================
- Coverage 85.76% 85.69% -0.08%
==========================================
Files 22 22
Lines 3604 3614 +10
==========================================
+ Hits 3091 3097 +6
- Misses 366 370 +4
Partials 147 147 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
integration/network_test.go (2)
33-38: Add assertion for sharding presence when explicitly set.When input sets Sharding, also assert network.Sharding.Valid() to ensure the field is actually present in the response, not just default false/true.
Apply this diff near the final assertions:
require.Equal(t, tt.wantSelf, network.Self.Value) require.Equal(t, tt.wantRemotes, network.Remotes.Value) require.Equal(t, tt.wantSharding, network.Sharding.Value) + if tt.input.Sharding.Valid() { + require.True(t, network.Sharding.Valid(), "sharding should be present in response when explicitly updated") + }
166-186: Standardize test names to writeApiKey (capital K).Minor consistency nit with the JSON field name.
- name: "add writeApiKey", + name: "add writeApiKey", @@ - name: "set writeApikey to empty", + name: "set writeApiKey to empty", @@ - name: "set writeApikey to null", + name: "set writeApiKey to null",Also applies to: 188-207, 209-228
types.go (1)
54-57: Consider redaction helpers for API keys.Optional: add String/GoString methods (or a MarshalText) on Remote that redact SearchAPIKey/WriteAPIKey to prevent accidental logs.
I can draft a minimal redactor if desired.
Also applies to: 59-81
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
integration/network_test.go(9 hunks)types.go(5 hunks)types_test.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
types_test.go (2)
types.go (2)
Network(20-24)Remote(53-57)field.go (3)
Null(58-58)Bool(15-15)String(21-21)
integration/network_test.go (2)
types.go (2)
Network(20-24)Remote(53-57)field.go (5)
Opt(49-52)NewOpt(55-55)String(21-21)Null(58-58)Bool(15-15)
types.go (2)
field.go (2)
Opt(49-52)Null(58-58)enum.go (2)
TaskStatus(6-6)TaskType(4-4)
⏰ 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). (2)
- GitHub Check: integration-tests (go latest version)
- GitHub Check: integration-tests (go current version)
🔇 Additional comments (7)
types_test.go (2)
191-204: Sharding marshaling tests look correct.Covers both explicit null and explicit true. Good guard for the new optional field.
249-258: writeApiKey marshaling tests are appropriate.Validates both set and explicit null cases for the new field.
integration/network_test.go (1)
80-92: Explicit Null on WriteAPIKey may unintentionally clear server state.In “update remote url only” and “update remote searchApiKey only” you send WriteAPIKey: Null. If the server treats null as “clear”, this is fine; if “omit to preserve,” consider omitting the field entirely to avoid accidental clearing.
Would you like me to adjust these cases to omit WriteAPIKey (not send) instead of sending null?
Also applies to: 100-112, 120-132
types.go (4)
21-24: Add Sharding to Network: LGTM.Field naming, type, and tag align with the API shape and Opt semantics.
41-45: Marshal sharding with Opt semantics: LGTM.Handles included vs null vs omitted correctly.
56-57: Add and marshal Remote.WriteAPIKey: LGTM.Consistent with SearchAPIKey behavior and satisfies response requirement to always include writeApiKey (value or null).
Also applies to: 74-78
304-317: Task adds network field: verify zero-value omitempty behavior.Using a value (not pointer) with
omitemptyis fine because both subfields default to nil. Keep if you don’t plan to distinguish “absent network” vs “present but empty”.If you need to detect absence vs empty explicitly, switch to
*TaskNetwork.
| Origin *Origin `json:"origin,omitempty"` | ||
| Remotes map[string]*TaskRemote `json:"remotes,omitempty"` | ||
| } | ||
|
|
||
| type Origin struct { | ||
| RemoteName string `json:"remoteName,omitempty"` | ||
| TaskUID string `json:"taskUid,omitempty"` | ||
| } | ||
|
|
||
| type TaskRemote struct { | ||
| TaskUID *string `json:"task_uid,omitempty"` | ||
| Error *string `json:"error,omitempty"` | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Inconsistent JSON casing and types for task UID fields under network.
- Origin.TaskUID uses
taskUid(camelCase) with type string. - TaskRemote.TaskUID uses
task_uid(snake_case) with type *string.
This diverges from the rest of the SDK (camelCase) and from Task.TaskUID (int64). Likely to cause unmarshal issues if the API returns a number. Recommend unifying to camelCase and int64.
Apply:
type TaskNetwork struct {
Origin *Origin `json:"origin,omitempty"`
Remotes map[string]*TaskRemote `json:"remotes,omitempty"`
}
type Origin struct {
RemoteName string `json:"remoteName,omitempty"`
- TaskUID string `json:"taskUid,omitempty"`
+ TaskUID int64 `json:"taskUid,omitempty"`
}
type TaskRemote struct {
- TaskUID *string `json:"task_uid,omitempty"`
+ TaskUID *int64 `json:"taskUid,omitempty"`
Error *string `json:"error,omitempty"`
}To confirm the upstream schema before merging, run:
If the Meilisearch 1.19.0 API truly uses snake_case or string UIDs here, I can adjust to a robust custom Unmarshal that accepts both strings and numbers.
🏁 Script executed:
#!/bin/bash
# Check for mixed casing/typing usage in the repo
rg -nP -C2 'task_uid|taskUid|TaskRemote|Origin' --type go
# Optional: ensure no other snake_case JSON tags remain for task fields
rg -nP 'json:"[a-z0-9_]*_[a-z0-9_]*"' --type goLength of output: 3617
Use int64 for TaskUID fields to match API numeric uids
JSON tags already follow the upstream spec (origin.taskUid and remotes.task_uid) and must remain unchanged. Change only the Go types to decode numeric UIDs correctly:
type Origin struct {
RemoteName string `json:"remoteName,omitempty"`
- TaskUID string `json:"taskUid,omitempty"`
+ TaskUID int64 `json:"taskUid,omitempty"`
}
type TaskRemote struct {
- TaskUID *string `json:"task_uid,omitempty"`
+ TaskUID *int64 `json:"task_uid,omitempty"`
Error *string `json:"error,omitempty"`
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Origin *Origin `json:"origin,omitempty"` | |
| Remotes map[string]*TaskRemote `json:"remotes,omitempty"` | |
| } | |
| type Origin struct { | |
| RemoteName string `json:"remoteName,omitempty"` | |
| TaskUID string `json:"taskUid,omitempty"` | |
| } | |
| type TaskRemote struct { | |
| TaskUID *string `json:"task_uid,omitempty"` | |
| Error *string `json:"error,omitempty"` | |
| } | |
| Origin *Origin `json:"origin,omitempty"` | |
| Remotes map[string]*TaskRemote `json:"remotes,omitempty"` | |
| } | |
| type Origin struct { | |
| RemoteName string `json:"remoteName,omitempty"` | |
| TaskUID int64 `json:"taskUid,omitempty"` | |
| } | |
| type TaskRemote struct { | |
| TaskUID *int64 `json:"task_uid,omitempty"` | |
| Error *string `json:"error,omitempty"` | |
| } |
🤖 Prompt for AI Agents
In types.go around lines 323 to 335, the TaskUID fields are defined as strings
but the API returns numeric UIDs; change the Go types to use int64 so JSON
decodes numeric UIDs correctly: replace Origin.TaskUID (currently string) with
int64 (or *int64 if you need nilability consistent with omitempty) and replace
TaskRemote.TaskUID (currently *string) with *int64, leaving all json tags
unchanged.
Pull Request
Related issue
Fixes #704
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
New Features
Tests