feat: wire noop as a first-class southbound type in CreateDataStore#456
Merged
Conversation
The noop target was already implemented but unreachable via the gRPC
CreateDataStore path — every request with type: noop was rejected with
"unknown targetconnection protocol type: noop" before a target was
ever constructed.
This change makes noop fully usable for CI/test pipelines:
- pkg/server: add case "noop" to the SBI-type switch so noop
datastore-creation requests are accepted; harden the default branch
to return a proper gRPC InvalidArgument status; fix long-standing
typo ("unknowm" → "unknown").
- pkg/datastore/target: remove the silent empty-type → noop fallback
so an omitted/empty SBI type now returns an InvalidArgument error
instead of silently becoming a noop target.
- pkg/datastore/target/noop: make AddSyncs a silent no-op (debug log
only) so callers can pass a full connection profile — including sync
config — without having to strip fields before the request.
All differences between target types are handled inside their own
packages; no if-type-==noop checks appear in generic infrastructure
code, consistent with the no-tight-coupling workspace rule.
New tests cover: noop acceptance and unknown-type rejection at the
server level, empty-type error and noop success in the target factory,
and AddSyncs / Get / Set / Status / Close behaviour in the noop package.
Co-authored-by: Cursor <cursoragent@cursor.com>
steiler
force-pushed
the
work/noop-target-connection
branch
from
7c4f74e to
a8680fa
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
noopinCreateDataStore— addscase "noop":to the SBI-type switch so noop datastore-creation requests are no longer rejected with"unknown targetconnection protocol type: noop"; hardens the default branch to return a proper gRPCInvalidArgumentstatus; fixes the long-standing"unknowm"typo.typenow returnsInvalidArgumentimmediately instead of silently constructing a noop target, making misconfiguration fail fast.noop.AddSyncsbecomes a silent no-op — callers can pass a full connection profile (address, port, TLS, credentials, sync config) without stripping fields; sync entries are accepted and discarded with a single debug-level log, consistent with how all other noop connection-profile fields are treated.All target-type differences stay inside their own packages — no
if type == noopchecks appear in generic infrastructure code, in line with theno-tight-couplingworkspace rule.