feat: Add Grpc registry store for Go feature server#362
Open
piket wants to merge 3 commits into
Open
Conversation
Adds a gRPC-backed registry store that mirrors the Python RemoteRegistry, allowing the Go feature server to connect directly to a Feast registry gRPC server without going through the HTTP translation layer. Key changes: - go/internal/feast/registry/grpc.go: GrpcRegistryStore using the existing RegistryServer gRPC stubs. Supports plain (grpc://) and TLS (grpcs://) connections, plus cert/is_tls config fields from Python RemoteRegistryConfig. - go/internal/feast/registry/registrystore.go: Add FallbackRegistryStore interface to replace *HttpRegistryStore type casts in registry.go. - go/internal/feast/registry/registry.go: Wire grpc/grpcs/remote schemes to GrpcRegistryStore in factory; replace 5 hard *HttpRegistryStore casts with FallbackRegistryStore interface assertions. Propagate errors on init for GrpcRegistryStore alongside FileRegistryStore and HttpRegistryStore. - go/internal/feast/registry/repoconfig.go: Add Cert and IsTls fields to RegistryConfig; parse registry_type: remote as GrpcRegistryStore. - go/internal/feast/registry/grpc_test.go: 10 tests using in-process bufconn server; 3 new config-parsing tests in repoconfig_test.go. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace hardcoded "remote" -> GrpcRegistryStore mapping with a lookup
against REGISTRY_STORE_CLASS_FOR_SCHEME so all scheme keys ("http",
"https", "grpc", "grpcs", "remote", "s3", "file") resolve correctly
via a single source of truth. Update all repoconfig tests to use
registry_type instead of registry_store_type.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
zabarn
reviewed
Jun 2, 2026
Comment on lines
+81
to
+91
| // parseGrpcTarget strips grpc:// or grpcs:// scheme prefixes and returns | ||
| // the target address along with whether TLS should be used. | ||
| func parseGrpcTarget(path string) (target string, useTLS bool) { | ||
| if strings.HasPrefix(path, "https://") { | ||
| return strings.TrimPrefix(path, "https://"), true | ||
| } | ||
| if strings.HasPrefix(path, "http://") { | ||
| return strings.TrimPrefix(path, "http://"), false | ||
| } | ||
| return path, false | ||
| } |
There was a problem hiding this comment.
I'm confused on this. I see the comment above is for it stripping grpc:// and grpcs://, but then the function only handles http and https
It should strip both based on what grpc.NewClient target, dialOpts...) expects as a target, right?
zabarn
reviewed
Jun 2, 2026
| dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials())) | ||
| } | ||
|
|
||
| conn, err := grpc.NewClient(target, dialOpts...) |
There was a problem hiding this comment.
HttpRegistryStore has a 5 second timeout applied here.
It looks like there's no timeouts set for this.
I think we can do it with this:
const defaultServiceConfig = `{
"methodConfig": [{
"name": [{}],
"timeout": "5s"
}]
}`
conn, err := grpc.NewClient(target,
dialOpts...,
grpc.WithDefaultServiceConfig(defaultServiceConfig),
)
https://pkg.go.dev/google.golang.org/grpc#WithDefaultServiceConfig
https://github.com/grpc/grpc/blob/master/doc/service_config.md
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.
What this PR does / why we need it
The Go feature server previously only supported
FileRegistryStore,HttpRegistryStore, andS3RegistryStore. Projects using a remote gRPC registry (Pythonregistry_type: remote) had nosupported Go-side equivalent, causing
GetOnlineFeaturesandGetOnlineFeaturesRangecalls tofail with
"sorted feature view X doesn't exist"— even when the view was correctly registered.This PR adds
GrpcRegistryStoreto the Go feature server, enabling it to connect to the Feastremote registry over gRPC with TLS.
Root cause of the "doesn't exist" error: All per-item fallback lookups in
registry.gowerehard-cast to
*HttpRegistryStore(e.g.r.registryStore.(*HttpRegistryStore).getSortedFeatureView(...)). This meant any non-HTTP storewould fail the type assertion at runtime, causing registry lookups to always return an error. This
PR introduces the
FallbackRegistryStoreinterface and replaces all five casts, so any storeimplementing
HasFallback() == truecan be used interchangeably.Changes
grpc.go— NewGrpcRegistryStorebacked byRegistryServerClient. TLS is enabled whenconfig.IsTlsis true,config.Certis set, or the path useshttps://. Supports system certpool or a custom PEM root CA for self-signed certs.
registrystore.go— NewFallbackRegistryStoreinterface abstracting per-item fetch methods(
getEntity,getFeatureView,getSortedFeatureView,getOnDemandFeatureView,getFeatureService).registry.go— Registers"remote"scheme →GrpcRegistryStorein the scheme map; addsGrpcRegistryStorecase togetRegistryStoreFromType; replaces all(*HttpRegistryStore)castswith
(FallbackRegistryStore).repoconfig.go— AddsCertandIsTlsfields toRegistryConfig; adds parsing forregistry_type(resolves through the existing scheme map),cert, andis_tlskeys.grpc_test.go— Full unit test suite usingbufconnfor in-process gRPC: covers all fetchmethods,
HasFallback,Teardown,UpdateRegistryProtonot-implemented, andparseGrpcTargetTLS scheme detection.
repoconfig_test.go— Extended with tests forremote,http,httpsregistry types;cert/is_tlsfields; defaultclient_id; and YAML-based config loading.Which issue(s) this PR fixes
Fixes the
"sorted feature view X doesn't exist"error on the Go feature server whenregistry_type: remoteis configured infeature_store.yaml.Testing
GOTOOLCHAIN=go1.23.12 go test ./go/internal/feast/registry/ -run TestGrpc -vmlpfs-range-query-perf-testtest env — all three registeredSortedFeatureViews resolve andGetOnlineFeaturesRangereaches Cassandra successfully