feat(grpc_datasource): add ConnectRPC transport for gRPC datasource#1480
Draft
fengyuwusong wants to merge 7 commits intowundergraph:masterfrom
Draft
feat(grpc_datasource): add ConnectRPC transport for gRPC datasource#1480fengyuwusong wants to merge 7 commits intowundergraph:masterfrom
fengyuwusong wants to merge 7 commits intowundergraph:masterfrom
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
5 tasks
Introduce RPCTransport interface to abstract the transport protocol in grpc_datasource, enabling both gRPC and Connect protocol support. Key changes: - New RPCTransport interface with grpcTransport and connectTransport impls - connectTransport implements Connect protocol over HTTP (Protobuf/JSON) - DataSource.cc refactored to DataSource.transport (RPCTransport) - NewDataSourceGRPC convenience function for backward compatibility - NewFactoryConnect in graphql_datasource for Connect protocol entry - ConnectConfiguration for Connect-specific settings - 6 unit tests for connectTransport (protobuf, json, errors, headers) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…t transport Keys ending in "-bin" carry binary values per gRPC convention. The Connect protocol requires these to be base64-encoded when placed in HTTP headers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…orts Address code review feedback: - Add nil check in grpcTransport.Invoke to prevent panics - Limit Connect response body to 10MB to prevent memory exhaustion Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…odies - Read maxSize+1 bytes to detect truncation and return a clear "response body exceeds N bytes" error instead of cryptic unmarshal failures - Truncate non-JSON error bodies to 256 chars in parseConnectError to prevent log bloat and sensitive data exposure Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the hand-written protoc invocation with `buf generate`, driven by new buf.yaml and buf.gen.yaml. The same configuration runs three plugins side by side: - protoc-gen-go (messages, unchanged from before) - protoc-gen-go-grpc (gRPC server/client, unchanged) - protoc-gen-connect-go (new: ConnectRPC handler/client) The Connect handler accepts gRPC, gRPC-Web, and Connect over the same HTTP endpoint, which lets the mock service serve both transports from a single registration. The generated code lives at productv1/productv1connect/product.connect.go. Both plugins use Mproduct.proto to remap the proto's go_package (cosmo/pkg/proto/productv1) onto the actual repository path so the generated Connect package can import productv1 cleanly.
…se base Two test sites added on master between v2.0.0-rc.265 (the original base of feat/connectrpc-datasource) and v2.0.0 still call NewDataSource(conn, ...). After the RPCTransport abstraction landed, NewDataSource expects an RPCTransport; gRPC connections must go through NewDataSourceGRPC. Update both call sites so the package builds against v2.0.0.
Add comprehensive end-to-end tests that drive the gRPC data source
pipeline through the new Connect transport against the existing
MockService.
- v2/pkg/grpctest/mockservice_connect.go:
Mechanically-generated MockServiceConnect adapter (one method per
RPC, 94 in total) that wraps the gRPC MockService onto the
productv1connect.ProductServiceHandler interface. The same backing
implementation can therefore serve Connect, gRPC, and gRPC-Web from
a single HTTP handler.
- v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_connect_test.go:
setupTestConnectServer spins the adapter up on httptest.NewUnstartedServer
+ h2c.NewHandler. Two new tests drive a representative query through
the data source via the Connect transport, once with Protobuf encoding
and once with JSON, to lock in wire-format parity for both encodings
end-to-end.
2d0024b to
3a0d79e
Compare
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
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.
@coderabbitai summary
Summary
This PR introduces an
RPCTransportabstraction inside the gRPC data source so that subgraph calls can be made over either native gRPC or ConnectRPC. It then adds the Connect implementation, regenerates thegrpctestfixture withbufso the same backingMockServicecan be served over both transports, and locks in end-to-end coverage through the data source layer.ConnectRPC is the right transport whenever end-to-end HTTP/2 to the subgraph is unavailable. The feature is consumed by the matching cosmo router work (see "Related").
Related
grpc_protocolconfiguration that picks the transport per subgraph).What's changed
Data source transport abstraction
RPCTransportinterface (transport.go) wraps the previous directgrpc.ClientConnInterfaceuse site so the data source can dispatch through any concrete transport.NewDataSource(transport, config)accepts the abstraction directly;NewDataSourceGRPC(conn, config)is a thin convenience wrapper that preserves the existing call shape for gRPC consumers.ConnectRPC transport
transport_connect.goimplements the Connect protocol over HTTP/1.1 with both Protobuf and JSON encodings.transport_connect_test.gocovers Protobuf, JSON, Connect-protocol errors, non-JSON errors, and ASCII / binary header forwarding.grpctest fixture migration to
bufbuf.yamlandbuf.gen.yamldriveprotoc-gen-go,protoc-gen-go-grpc, andprotoc-gen-connect-gofrom a single generation step. TheMproduct.proto=plugin option remaps the proto'sgo_packageonto the actual repository path so the generated Connect package can importproductv1cleanly.Makefilegenerate-prototarget now invokesbuf generate.productv1/productv1connect/product.connect.gois checked in.Connect-compatible MockService
pkg/grpctest/mockservice_connect.go: mechanically generatedMockServiceConnectadapter that forwards every Connect call to the existingMockService(one method per RPC, 94 in total). The same backing implementation can now serve Connect, gRPC, and gRPC-Web from a single HTTP handler.Data source end-to-end coverage
pkg/engine/datasource/grpc_datasource/grpc_datasource_connect_test.go: drives the data source through the new Connect transport against the Connect-wrappedMockServicefor both Protobuf and JSON encodings.masterbetween rebase points (callingNewDataSource(conn, …)) ontoNewDataSourceGRPC(conn, …).Testing
pkg/engine/datasource/grpc_datasource: 456 tests pass (Connect cases added; existing gRPC / federation / cost / lookup tests unchanged).pkg/grpctest: package builds with both protoc and buf-generated outputs in sync.Backward compatibility
RPCTransportintroduction keepsNewDataSourceGRPCas the drop-in replacement for the oldNewDataSource(conn, …)call shape; existing gRPC consumers do not need to touch anything else.productv1/productv1connectsub-package so they don't affect existing imports ofproductv1.Checklist
docs-website/router/...).Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.