Skip to content

treat delete path not found as success#439

Open
henderiw wants to merge 1 commit into
mainfrom
delete-not-found
Open

treat delete path not found as success#439
henderiw wants to merge 1 commit into
mainfrom
delete-not-found

Conversation

@henderiw
Copy link
Copy Markdown
Contributor

@henderiw henderiw commented May 6, 2026

handle delete path not found as success

@henderiw henderiw requested a review from a team as a code owner May 6, 2026 09:01
}
needle := ops.ErrNavigateSdcpbPathNotFound.Error()
for _, e := range intentErrors {
if !strings.Contains(e, needle) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we not able to use errors.Is or other errors.* functions? do we get the errors as anything but strings?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@steiler if you expose these errors in proto files this might be the better solution.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean exactly? expose an error in the sdcpb repo, as a golang error or something proto specific?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally we have transport errors that are covered in the gRPC transport layer, but these are application errors we need to expose in the proto as part of the result or so.

Comment thread go.mod
github.com/prometheus/client_golang v1.23.2
github.com/prometheus/client_model v0.6.2
github.com/prometheus/prometheus v0.309.1
github.com/sdcio/data-server v0.0.69-0.20260506072726-3d7d03086dfa
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should decide if we really want to deal with a strict import of data-server in config-server

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we will but so far there is no tagged version that has this capability and the alternative is defining our own errors.

@github-project-automation github-project-automation Bot moved this to Backlog in SDC project May 11, 2026
steiler added a commit to sdcio/data-server that referenced this pull request May 22, 2026
Fixes **idempotent delete** when the target path is **already absent** from the internal config tree: `writeBackSyncTree` → `DeleteBranch` no longer fails on `NavigateSdcpbPath` “path not found”, so **TransactionSet** can complete successfully instead of surfacing a hard RPC error that **config-server** treated as unrecoverable.
**Domain:** `ops.ErrNavigateSdcpbPathNotFound` stays in **data-server**; `DeleteBranch` treats it as **no-op** (aligned with existing `ApplyToRunning` / `sync` behavior). **No** extra gRPC mapping or **sdc-protos** wire helpers.
**Relation to [config-server#439](sdcio/config-server#439) / [#433](sdcio/config-server#433 addresses the **root cause** so string-matching or importing **data-server** errors in config-server for this case should be **unnecessary** once this server version is in use.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

Delete intents should treat backend 'path not found in tree' as idempotent success

3 participants