Copilot commented on code in PR #1215:
URL:
https://github.com/apache/skywalking-banyandb/pull/1215#discussion_r3557965859
##########
banyand/metadata/schema/property/client.go:
##########
@@ -862,6 +930,9 @@ func (r *SchemaRegistry) UpdateTrace(ctx context.Context,
trace *databasev1.Trac
if validateErr := validate.Trace(trace); validateErr != nil {
return 0, validateErr
}
+ if pipelineErr := r.checkTraceAgainstPipeline(ctx, trace); pipelineErr
!= nil {
+ return 0, pipelineErr
+ }
Review Comment:
In UpdateTrace, the sampler-pipeline single-trace check runs **before**
updateResource verifies that the trace schema exists. This can cause
`UpdateTrace` on a non-existent trace (e.g., updating "trace-b" when only
"trace-a" exists in a sampler-enabled group) to return
`codes.FailedPrecondition` instead of a not-found error, which is misleading
and changes error semantics.
Move the cross-entity check into the `updateResource` validator so it
executes only after the existing trace has been loaded/validated.
##########
banyand/metadata/schema/property/client.go:
##########
@@ -904,6 +978,9 @@ func (r *SchemaRegistry) UpdateGroup(ctx context.Context,
group *commonv1.Group)
if validateErr := validate.Group(group); validateErr != nil {
return 0, validateErr
}
+ if pipelineErr := r.checkPipelineAgainstTraces(ctx, group); pipelineErr
!= nil {
+ return 0, pipelineErr
+ }
Review Comment:
In UpdateGroup, the sampler-pipeline single-trace check runs before
`updateResource` verifies the group exists. If a caller attempts to update a
non-existent group (or one that failed to load) and there happen to be multiple
traces under that name, this can return `codes.FailedPrecondition` instead of a
not-found error.
Run `checkPipelineAgainstTraces` as an `updateResource` validator so it only
executes after the existing group schema has been confirmed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
