feat: Allow partial success results - #400
Conversation
91aa4ef to
e0a2d5b
Compare
nurof3n
changed the title
67276b3 to
1d4c4d4
Compare
|
Ho hum. Can I get an explanation of what this one is for? 🤔 It's not immediately obvious to me sorry! |
@jedevc Yup, so the router can return a 207 on listing or bulk requests when some of the metros returned errors, and it would contain both the results for successful calls and the errors for the others. |
Signed-off-by: Alex-Andrei Cioc <andrei.cioc@unikraft.io>
Signed-off-by: Alex-Andrei Cioc <andrei.cioc@unikraft.io>
1d4c4d4 to
48a0314
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the CLI’s multi-metro list/get/delete operations to allow returning partial results when the API returns both a response payload and an error (or when only some items succeed). It introduces shared helpers in internal/cmd/util.go and wires them into several resource commands (instances, volumes, templates, certificates, service groups), aligning with TOOL-1048 and the follow-up to PR #388.
Changes:
- Add shared partial-success helpers and a
PartialResulterror type to preserve successful items while still surfacing failures. - Update list/get handlers across multiple resources to continue processing response payloads even when an API error is present (when data exists).
- Update delete handlers to detect partial deletions and return structured partial failures when possible.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/cmd/util.go | Adds partial-success helpers (listGetOpError, deleteOpError), PartialResult, and error-combining utilities. |
| internal/cmd/instances.go | Uses partial-success logic for list/get and returns partial delete results when some instances delete successfully. |
| internal/cmd/instance_templates.go | Uses partial-success logic for template instance list/get/delete operations. |
| internal/cmd/volumes.go | Uses partial-success logic for volume list/get and returns partial delete results when possible. |
| internal/cmd/volume_templates.go | Uses partial-success logic for template volume list/get/delete operations. |
| internal/cmd/certificates.go | Uses partial-success logic for certificate list/get and returns partial delete results when possible. |
| internal/cmd/services.go | Uses partial-success logic for service group list/get; delete path attempts partial handling but currently has dead/unreachable logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if instances != nil { | ||
| for _, instance := range instances.Data.Instances { | ||
| if instance.Status == platform.ResponseStatusSuccess { |
Signed-off-by: Alex-Andrei Cioc <andrei.cioc@unikraft.io>
48a0314 to
7aece91
Compare
Makes sense. The bit that I don't get is the new helpers in |
| return deleted, nil | ||
| // If there were deletions despite the error, report partial success | ||
| if len(deleted) > 0 && err != nil && !platform.ErrorContainsOnly(err, platform.APIHTTPErrorNotFound) { | ||
| pr := NewPartialResult() |
There was a problem hiding this comment.
We should match how Get already handles this? And just use errors.Join.
Signed-off-by: Alex-Andrei Cioc <andrei.cioc@unikraft.io>
To be merged after #388
Closes TOOL-1048