UPSTREAM: <carry>: Add GetCustomSchema OPM serve gRPC endpoint#1296
UPSTREAM: <carry>: Add GetCustomSchema OPM serve gRPC endpoint#1296perdasilva wants to merge 1 commit into
Conversation
Signed-off-by: Per G. da Silva <pegoncal@redhat.com>
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: perdasilva The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
WalkthroughA new ChangesCustom Schema Streaming
Sequence DiagramsequenceDiagram
participant Client
participant Server as gRPC Server
participant CacheIface as Cache Interface
participant Backend as Cache Backend
Client->>Server: GetCustomSchemas(schema, packageName, name)
Server->>Server: Check if store supports<br/>customSchemaQuerier
alt unsupported
Server-->>Client: Unimplemented error
else supported
Server->>CacheIface: SendCustomSchemas(ctx, filters...,<br/>sender callback)
CacheIface->>Backend: SendMetas(ctx, sender)
activate Backend
loop for each stored meta key
Backend->>Backend: Read blob from storage
Backend->>CacheIface: sender(schema, pkg, name, blob)
end
deactivate Backend
CacheIface->>Server: return error (if any)
Server->>Server: Unmarshal JSON blob<br/>to map[string]interface{}
Server->>Server: Convert map to<br/>protobuf Struct
Server-->>Client: Stream Struct
end
Client->>Client: Receive stream of<br/>custom schema Structs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 4
🧹 Nitpick comments (1)
staging/operator-registry/pkg/server/server_test.go (1)
812-815: ⚡ Quick winCover the unfiltered RPC path against the populated custom-schema cache.
Right now the empty-request case only runs against
cacheAddress, so it would still pass if the handler accidentally treated an emptySchemaas “return nothing.” Adding an empty-schema case againstcustomSchemaCacheAddressand asserting the two distinct resource names would make this new RPC coverage much harder to fool.Also applies to: 827-841, 850-862
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@staging/operator-registry/pkg/server/server_test.go` around lines 812 - 815, Add an empty-request test run that targets the populated custom-schema cache: in TestGetCustomSchemas call testGetCustomSchemasEmpty(customSchemaCacheAddress) (in addition to the existing cacheAddress case) and assert the response returns the two distinct resource names expected; likewise update the related test runs referenced (the blocks around lines 827–841 and 850–862) to invoke the empty-schema path against customSchemaCacheAddress as well so the unfiltered RPC path is exercised and the two resource names are asserted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@staging/operator-registry/pkg/cache/json.go`:
- Around line 198-205: SendMetas currently discards the context parameter;
update jsonBackend.SendMetas to honor the provided ctx by checking ctx.Err() (or
ctx.Done()) inside the metas.Walk callback (before reading the file and before
calling sender) and return early with the context error when cancelled; locate
the jsonBackend.SendMetas function and its anonymous callback passed to
q.metas.Walk, add a context cancellation check at the top of that callback and
propagate ctx.Err() (or wrap it) instead of continuing to use q.metaFile,
os.ReadFile, or sender when cancelled.
In `@staging/operator-registry/pkg/cache/pogrebv1.go`:
- Around line 214-221: The SendMetas function currently ignores its context (it
uses _ context.Context) so pogreb scanning continues after cancellation; change
the signature to use the provided ctx and in the q.metas.Walk callback check
ctx.Err() before doing work (e.g., at the top of the Walk func) and return
ctx.Err() to stop the walk when canceled; ensure this check happens before
q.db.Get(q.metaDBKey(key)) and before calling sender(key.Schema,
key.PackageName, key.Name, data) so DB reads and sends stop promptly.
In `@staging/operator-registry/pkg/server/server.go`:
- Around line 117-129: The fmt.Errorf calls inside the lambda passed to
mq.SendCustomSchemas are not wrapping errors with %w; update the error returns
in the anonymous function (the json.Unmarshal error and the structpb.NewStruct
error) to use fmt.Errorf with %w so callers can inspect the underlying error
(i.e., change the two fmt.Errorf("%v", err) uses to use a descriptive message
and %w, keeping the same surrounding text in the function passed to
mq.SendCustomSchemas that ultimately calls stream.Send).
---
Nitpick comments:
In `@staging/operator-registry/pkg/server/server_test.go`:
- Around line 812-815: Add an empty-request test run that targets the populated
custom-schema cache: in TestGetCustomSchemas call
testGetCustomSchemasEmpty(customSchemaCacheAddress) (in addition to the existing
cacheAddress case) and assert the response returns the two distinct resource
names expected; likewise update the related test runs referenced (the blocks
around lines 827–841 and 850–862) to invoke the empty-schema path against
customSchemaCacheAddress as well so the unfiltered RPC path is exercised and the
two resource names are asserted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b75bca00-e245-487b-a36c-36f55ccac912
⛔ Files ignored due to path filters (10)
staging/operator-registry/pkg/api/registry.pb.gois excluded by!**/*.pb.gostaging/operator-registry/pkg/api/registry_grpc.pb.gois excluded by!**/*.pb.govendor/github.com/operator-framework/operator-registry/pkg/api/registry.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/api/registry.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/api/registry_grpc.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/cache/cache.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/cache/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/cache/meta_key.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/cache/pogrebv1.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/operator-framework/operator-registry/pkg/server/server.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (13)
staging/operator-registry/AGENTS.mdstaging/operator-registry/Makefilestaging/operator-registry/README.mdstaging/operator-registry/pkg/api/registry.protostaging/operator-registry/pkg/cache/cache.gostaging/operator-registry/pkg/cache/cache_test.gostaging/operator-registry/pkg/cache/json.gostaging/operator-registry/pkg/cache/meta_key.gostaging/operator-registry/pkg/cache/pogrebv1.gostaging/operator-registry/pkg/client/client_test.gostaging/operator-registry/pkg/server/server.gostaging/operator-registry/pkg/server/server_test.gostaging/operator-registry/scripts/ensure-protoc.sh
| // defaultMetaName is used as the metaKey name for custom schema blobs that | ||
| // lack a "name" field. Since each (schema, packageName) pair must be unique, | ||
| // a collision on this sentinel indicates duplicate data in the FBC source. | ||
| const defaultMetaName = "noname" |
There was a problem hiding this comment.
defaultMetaName can alias a real custom-schema entry.
A nameless blob and a blob explicitly named "noname" in the same (schema, package) collapse to the same metaKey, so this can raise a false duplicate custom schema meta error or make lookup ambiguous. Please encode “unnamed” separately or reserve/reject the sentinel for user-provided names.
Also applies to: 321-337
| func (q *jsonBackend) SendMetas(_ context.Context, sender func(schema, pkg, name string, blob []byte) error) error { | ||
| return q.metas.Walk(func(key metaKey) error { | ||
| data, err := os.ReadFile(q.metaFile(key)) | ||
| if err != nil { | ||
| return fmt.Errorf("get meta %v: %w", key, err) | ||
| } | ||
| return sender(key.Schema, key.PackageName, key.Name, data) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual code at the specified lines
head -n 210 staging/operator-registry/pkg/cache/json.go | tail -n 20Repository: openshift/operator-framework-olm
Length of output: 689
🏁 Script executed:
# Let's get more context around the function
sed -n '195,210p' staging/operator-registry/pkg/cache/json.goRepository: openshift/operator-framework-olm
Length of output: 575
🏁 Script executed:
# Let's find the Walk function definition to understand if it already supports context
rg "func.*Walk" staging/operator-registry/pkg/cache/json.go -A 3Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
# Check the type of metas field and its Walk method
rg "metas\s*:" staging/operator-registry/pkg/cache/json.go -B 5 -A 2Repository: openshift/operator-framework-olm
Length of output: 229
🏁 Script executed:
# Look for the entire jsonBackend struct
ast-grep --pattern 'type jsonBackend struct { $$$ }'Repository: openshift/operator-framework-olm
Length of output: 855
🏁 Script executed:
# Find metaKeys type definition
rg "type metaKeys" staging/operator-registry/pkg/cache/json.go -A 5Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
# Search for Walk method definition on metaKeys
rg "func.*metaKeys.*Walk" staging/operator-registry/pkg/cache/json.go -A 8Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
# More general search for Walk method
rg "func.*\(.*metaKeys\).*Walk" staging/operator-registry/pkg/cache/json.go -A 8Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
# Search for any Walk method in the file
rg "func.*Walk" staging/operator-registry/pkg/cache/ -A 5Repository: openshift/operator-framework-olm
Length of output: 996
🏁 Script executed:
# Get full implementation of metaKeys.Walk
sed -n '/^func (m \*metaKeys) Walk/,/^func /p' staging/operator-registry/pkg/cache/meta_key.go | head -n 20Repository: openshift/operator-framework-olm
Length of output: 364
🏁 Script executed:
# Get full implementation of bundleKeys.Walk
sed -n '/^func (b bundleKeys) Walk/,/^func /p' staging/operator-registry/pkg/cache/bundle_key.go | head -n 20Repository: openshift/operator-framework-olm
Length of output: 245
🏁 Script executed:
# Check if there are similar Send* functions in other backends (file.go, sql.go, etc)
fd "\\.go$" staging/operator-registry/pkg/cache/ --exec grep -l "Send" {} \;Repository: openshift/operator-framework-olm
Length of output: 263
🏁 Script executed:
# Look for other Send* functions that accept context
rg "func.*Send.*context.Context" staging/operator-registry/pkg/cache/ -A 3Repository: openshift/operator-framework-olm
Length of output: 2542
🏁 Script executed:
# Find the interface that defines SendMetas
rg "SendMetas" staging/operator-registry/pkg/cache/ -B 3 | grep -E "(interface|type.*struct)" -A 10Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
# Search for Backend interface definition
rg "type.*Backend" staging/operator-registry/pkg/cache/ | head -20Repository: openshift/operator-framework-olm
Length of output: 224
🏁 Script executed:
# Look at cache.go to see the Backend interface
sed -n '1,100p' staging/operator-registry/pkg/cache/cache.go | grep -A 30 "interface"Repository: openshift/operator-framework-olm
Length of output: 1281
🏁 Script executed:
# Search for Backend interface in cache.go more thoroughly
rg "interface" staging/operator-registry/pkg/cache/cache.go -A 20Repository: openshift/operator-framework-olm
Length of output: 1091
🏁 Script executed:
# Check SendBundles implementation in json.go
sed -n '/func (q \*jsonBackend) SendBundles/,/^func /p' staging/operator-registry/pkg/cache/json.go | head -30Repository: openshift/operator-framework-olm
Length of output: 1070
🏁 Script executed:
# Check SendBundles implementation in pogrebv1.go
sed -n '/func (q \*pogrebV1Backend) SendBundles/,/^func /p' staging/operator-registry/pkg/cache/pogrebv1.go | head -30Repository: openshift/operator-framework-olm
Length of output: 670
Use the context parameter to cancel iteration on context cancellation.
The SendMetas method accepts context.Context per the interface contract but explicitly discards it with _. Since this function iterates over potentially many metadata blobs and reads each from disk, it should respect context cancellation—particularly when backing streamed RPCs. Add context error checks in the loop to abort early if the caller cancels.
Suggested change
-func (q *jsonBackend) SendMetas(_ context.Context, sender func(schema, pkg, name string, blob []byte) error) error {
+func (q *jsonBackend) SendMetas(ctx context.Context, sender func(schema, pkg, name string, blob []byte) error) error {
return q.metas.Walk(func(key metaKey) error {
+ if err := ctx.Err(); err != nil {
+ return err
+ }
data, err := os.ReadFile(q.metaFile(key))
if err != nil {
return fmt.Errorf("get meta %v: %w", key, err)
}
+ if err := ctx.Err(); err != nil {
+ return err
+ }
return sender(key.Schema, key.PackageName, key.Name, data)
})
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@staging/operator-registry/pkg/cache/json.go` around lines 198 - 205,
SendMetas currently discards the context parameter; update jsonBackend.SendMetas
to honor the provided ctx by checking ctx.Err() (or ctx.Done()) inside the
metas.Walk callback (before reading the file and before calling sender) and
return early with the context error when cancelled; locate the
jsonBackend.SendMetas function and its anonymous callback passed to
q.metas.Walk, add a context cancellation check at the top of that callback and
propagate ctx.Err() (or wrap it) instead of continuing to use q.metaFile,
os.ReadFile, or sender when cancelled.
| func (q *pogrebV1Backend) SendMetas(_ context.Context, sender func(schema, pkg, name string, blob []byte) error) error { | ||
| return q.metas.Walk(func(key metaKey) error { | ||
| data, err := q.db.Get(q.metaDBKey(key)) | ||
| if err != nil { | ||
| return fmt.Errorf("get meta %v: %w", key, err) | ||
| } | ||
| return sender(key.Schema, key.PackageName, key.Name, data) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n staging/operator-registry/pkg/cache/pogrebv1.go | head -250 | tail -50Repository: openshift/operator-framework-olm
Length of output: 1733
🏁 Script executed:
wc -l staging/operator-registry/pkg/cache/pogrebv1.goRepository: openshift/operator-framework-olm
Length of output: 128
🏁 Script executed:
cat -n staging/operator-registry/pkg/cache/pogrebv1.go | sed -n '200,230p'Repository: openshift/operator-framework-olm
Length of output: 1226
🏁 Script executed:
rg -A 10 "func.*Walk" staging/operator-registry/pkg/cache/pogrebv1.goRepository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
rg "func.*Backend.*Send" staging/operator-registry/pkg/cache/pogrebv1.goRepository: openshift/operator-framework-olm
Length of output: 287
🏁 Script executed:
fd "*.go" staging/operator-registry/pkg/cache/ | head -10Repository: openshift/operator-framework-olm
Length of output: 360
🏁 Script executed:
rg -A 8 "func.*SendMetas" staging/operator-registry/pkg/cache/Repository: openshift/operator-framework-olm
Length of output: 1603
🏁 Script executed:
rg -B 5 -A 15 "func (q \*pogrebV1Backend) SendMetas" staging/operator-registry/pkg/cache/pogrebv1.goRepository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
rg "\.Walk\(" staging/operator-registry/pkg/cache/pogrebv1.goRepository: openshift/operator-framework-olm
Length of output: 290
🏁 Script executed:
rg -B 2 -A 10 "metas\.Walk" staging/operator-registry/pkg/cache/pogrebv1.goRepository: openshift/operator-framework-olm
Length of output: 567
🏁 Script executed:
cat -n staging/operator-registry/pkg/cache/pogrebv1.go | sed -n '1,50p'Repository: openshift/operator-framework-olm
Length of output: 1404
🏁 Script executed:
cat -n staging/operator-registry/pkg/cache/pogrebv1.go | sed -n '100,150p'Repository: openshift/operator-framework-olm
Length of output: 1661
🏁 Script executed:
rg "metas" staging/operator-registry/pkg/cache/pogrebv1.go | head -20Repository: openshift/operator-framework-olm
Length of output: 321
🏁 Script executed:
rg -B 5 -A 10 "type metaKeys" staging/operator-registry/pkg/cache/Repository: openshift/operator-framework-olm
Length of output: 1128
🏁 Script executed:
rg -B 2 -A 15 "func.*metaKeys.*Walk" staging/operator-registry/pkg/cache/Repository: openshift/operator-framework-olm
Length of output: 1231
🏁 Script executed:
rg -B 2 -A 15 "func.*SendBundles" staging/operator-registry/pkg/cache/pogrebv1.goRepository: openshift/operator-framework-olm
Length of output: 673
🏁 Script executed:
rg -B 2 -A 15 "func.*SendBundles" staging/operator-registry/pkg/cache/json.goRepository: openshift/operator-framework-olm
Length of output: 697
🏁 Script executed:
rg "ctx\.Err\(\)" staging/operator-registry/pkg/cache/Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
rg "func.*Send" staging/operator-registry/pkg/cache/ | grep -v "BundleSender\|MetaSender"Repository: openshift/operator-framework-olm
Length of output: 932
🏁 Script executed:
rg "func.*Send.*_ context.Context" staging/operator-registry/pkg/cache/Repository: openshift/operator-framework-olm
Length of output: 674
🏁 Script executed:
rg -B 5 "SendMetas\|SendBundles" staging/operator-registry/pkg/cache/backend.goRepository: openshift/operator-framework-olm
Length of output: 163
🏁 Script executed:
rg -B 3 -A 10 "type backend interface" staging/operator-registry/pkg/cache/Repository: openshift/operator-framework-olm
Length of output: 1066
🏁 Script executed:
rg -B 3 -A 3 "select.*ctx\.Done\|if.*ctx\.Err" staging/operator-registry/pkg/ | head -40Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
fd registry.go staging/operator-registry/pkg/ | head -3Repository: openshift/operator-framework-olm
Length of output: 280
🏁 Script executed:
rg "SendMetas\|SendBundles" staging/operator-registry/pkg/registry/ | head -20Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
rg -A 30 "type backend interface" staging/operator-registry/pkg/cache/cache.goRepository: openshift/operator-framework-olm
Length of output: 948
🏁 Script executed:
rg -B 5 -A 8 "SendMetas" staging/operator-registry/pkg/registry/ | head -50Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
rg "\.SendMetas\|\.SendBundles" staging/operator-registry/ --type go | head -30Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
rg "SendMetas\|SendBundles" staging/operator-registry/ --type go | grep -E "test\.go|example|handler" | head -20Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
fd "service.pb.go\|service.go" staging/operator-registry/pkg/api/ | head -5Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
rg "Metas\|Bundles" staging/operator-registry/pkg/api/ | grep "func\|Sender" | head -20Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
rg "ListBundles\|GetBundles" staging/operator-registry/pkg/ --type go | head -10Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
fd "\.proto" staging/operator-registry/Repository: openshift/operator-framework-olm
Length of output: 125
🏁 Script executed:
rg "rpc.*Meta\|rpc.*Bundle" staging/operator-registry/ | head -20Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
rg -B 5 -A 10 "SendMetas\|SendBundles" staging/operator-registry/pkg/cache/cache.goRepository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
rg "backend\|\.SendMetas\|\.SendBundles" staging/operator-registry/pkg/cache/cache.go | head -40Repository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
cat -n staging/operator-registry/pkg/cache/cache.go | head -100Repository: openshift/operator-framework-olm
Length of output: 3390
🏁 Script executed:
rg "func.*cache.*Send" staging/operator-registry/pkg/cache/cache.goRepository: openshift/operator-framework-olm
Length of output: 318
🏁 Script executed:
cat -n staging/operator-registry/pkg/cache/json.go | sed -n '140,180p'Repository: openshift/operator-framework-olm
Length of output: 1459
🏁 Script executed:
cat -n staging/operator-registry/pkg/cache/pogrebv1.go | sed -n '260,290p'Repository: openshift/operator-framework-olm
Length of output: 1187
🏁 Script executed:
rg -B 5 -A 10 "c\.backend\.SendMetas\|backend\.SendMetas" staging/operator-registry/pkg/cache/cache.goRepository: openshift/operator-framework-olm
Length of output: 58
🏁 Script executed:
cat -n staging/operator-registry/pkg/cache/cache.go | sed -n '200,300p'Repository: openshift/operator-framework-olm
Length of output: 4497
🏁 Script executed:
rg "func.*cache.*" staging/operator-registry/pkg/cache/cache.go | head -20Repository: openshift/operator-framework-olm
Length of output: 1677
🏁 Script executed:
cat -n staging/operator-registry/pkg/cache/cache.go | sed -n '251,264p'Repository: openshift/operator-framework-olm
Length of output: 606
Honor cancellation while scanning Pogreb meta entries.
SendMetas keeps reading from Pogreb even after the request context is canceled. For a streamed gRPC path, that means unnecessary DB work after the client is already gone.
Suggested change
-func (q *pogrebV1Backend) SendMetas(_ context.Context, sender func(schema, pkg, name string, blob []byte) error) error {
+func (q *pogrebV1Backend) SendMetas(ctx context.Context, sender func(schema, pkg, name string, blob []byte) error) error {
return q.metas.Walk(func(key metaKey) error {
+ if err := ctx.Err(); err != nil {
+ return err
+ }
data, err := q.db.Get(q.metaDBKey(key))
if err != nil {
return fmt.Errorf("get meta %v: %w", key, err)
}
+ if err := ctx.Err(); err != nil {
+ return err
+ }
return sender(key.Schema, key.PackageName, key.Name, data)
})
}📝 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.
| func (q *pogrebV1Backend) SendMetas(_ context.Context, sender func(schema, pkg, name string, blob []byte) error) error { | |
| return q.metas.Walk(func(key metaKey) error { | |
| data, err := q.db.Get(q.metaDBKey(key)) | |
| if err != nil { | |
| return fmt.Errorf("get meta %v: %w", key, err) | |
| } | |
| return sender(key.Schema, key.PackageName, key.Name, data) | |
| }) | |
| func (q *pogrebV1Backend) SendMetas(ctx context.Context, sender func(schema, pkg, name string, blob []byte) error) error { | |
| return q.metas.Walk(func(key metaKey) error { | |
| if err := ctx.Err(); err != nil { | |
| return err | |
| } | |
| data, err := q.db.Get(q.metaDBKey(key)) | |
| if err != nil { | |
| return fmt.Errorf("get meta %v: %w", key, err) | |
| } | |
| if err := ctx.Err(); err != nil { | |
| return err | |
| } | |
| return sender(key.Schema, key.PackageName, key.Name, data) | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@staging/operator-registry/pkg/cache/pogrebv1.go` around lines 214 - 221, The
SendMetas function currently ignores its context (it uses _ context.Context) so
pogreb scanning continues after cancellation; change the signature to use the
provided ctx and in the q.metas.Walk callback check ctx.Err() before doing work
(e.g., at the top of the Walk func) and return ctx.Err() to stop the walk when
canceled; ensure this check happens before q.db.Get(q.metaDBKey(key)) and before
calling sender(key.Schema, key.PackageName, key.Name, data) so DB reads and
sends stop promptly.
| return mq.SendCustomSchemas(stream.Context(), req.GetSchema(), req.GetPackageName(), req.GetName(), | ||
| func(_, _, _ string, blob []byte) error { | ||
| var m map[string]interface{} | ||
| if err := json.Unmarshal(blob, &m); err != nil { | ||
| return fmt.Errorf("unmarshal custom schema blob: %v", err) | ||
| } | ||
| s, err := structpb.NewStruct(m) | ||
| if err != nil { | ||
| return fmt.Errorf("convert custom schema blob to struct: %v", err) | ||
| } | ||
| return stream.Send(s) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Wrap errors with %w for proper error chain propagation.
Lines 121 and 125 use fmt.Errorf with %v instead of %w. This breaks the error chain and prevents callers from using errors.Is or errors.As to inspect the underlying error.
🔧 Proposed fix
var m map[string]interface{}
if err := json.Unmarshal(blob, &m); err != nil {
- return fmt.Errorf("unmarshal custom schema blob: %v", err)
+ return fmt.Errorf("unmarshal custom schema blob: %w", err)
}
s, err := structpb.NewStruct(m)
if err != nil {
- return fmt.Errorf("convert custom schema blob to struct: %v", err)
+ return fmt.Errorf("convert custom schema blob to struct: %w", err)
}As per coding guidelines: "Wrap errors with context using fmt.Errorf() with the %w verb in Go code" for files matching staging/operator-registry/**/*.go.
📝 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.
| return mq.SendCustomSchemas(stream.Context(), req.GetSchema(), req.GetPackageName(), req.GetName(), | |
| func(_, _, _ string, blob []byte) error { | |
| var m map[string]interface{} | |
| if err := json.Unmarshal(blob, &m); err != nil { | |
| return fmt.Errorf("unmarshal custom schema blob: %v", err) | |
| } | |
| s, err := structpb.NewStruct(m) | |
| if err != nil { | |
| return fmt.Errorf("convert custom schema blob to struct: %v", err) | |
| } | |
| return stream.Send(s) | |
| }) | |
| } | |
| return mq.SendCustomSchemas(stream.Context(), req.GetSchema(), req.GetPackageName(), req.GetName(), | |
| func(_, _, _ string, blob []byte) error { | |
| var m map[string]interface{} | |
| if err := json.Unmarshal(blob, &m); err != nil { | |
| return fmt.Errorf("unmarshal custom schema blob: %w", err) | |
| } | |
| s, err := structpb.NewStruct(m) | |
| if err != nil { | |
| return fmt.Errorf("convert custom schema blob to struct: %w", err) | |
| } | |
| return stream.Send(s) | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@staging/operator-registry/pkg/server/server.go` around lines 117 - 129, The
fmt.Errorf calls inside the lambda passed to mq.SendCustomSchemas are not
wrapping errors with %w; update the error returns in the anonymous function (the
json.Unmarshal error and the structpb.NewStruct error) to use fmt.Errorf with %w
so callers can inspect the underlying error (i.e., change the two
fmt.Errorf("%v", err) uses to use a descriptive message and %w, keeping the same
surrounding text in the function passed to mq.SendCustomSchemas that ultimately
calls stream.Send).
1 participant
Summary by CodeRabbit
New Features
GetCustomSchemasgRPC streaming endpoint to query custom schema metadata from the registry.Documentation