-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(xds): add public APIs for xds-client #2464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@YutaoMa CI is failing because we have a job that checks that we don't expose certain APIs publically so this should be addressed https://github.com/hyperium/tonic/actions/runs/20112284748/job/57712601774?pr=2464#step:7:69 ideally, we should not expose things publically that we don't own to avoid potential breaking changes in the future. |
|
@LucioFranco as suggested I've removed the dependency on the pre-1.0 |
gu0keno0
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some general questions, LGTM overall
LucioFranco
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Feel free to address things in follow ups, nice to see the start here!
dfawley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review. In general this looks great, thanks! I have a few changes to suggest, and feel free to punt on some of them until later.
## Motivation Ref: #2444 The transport layer in xDS client is responsible for sending `DiscoveryRequest` to xDS management server, and receiving `DiscoveryResponse` back. ## Solution This PR implements tonic-based transport and prost-based codec. As discussed in #2464, the transport layer sends and receives bytes and leaves (de)serialization to the worker with the help of codegen-specific codecs.
Motivation
As explained previously #2459 , we are upstreaming an implementation of xDS client in Rust, in order to support xDS based routing and load balancing in tonic / gRPC Rust eventually. This PR is a continuation of the effort, by exposing all the expected public APIs (traits, types, function signatures) first to lay out the design.
Solution
Here is an overview of all the public APIs we are adding in this PR:
error.rsError,Resultresource.rsResourcetraittransport.rsTransport,TransportStream,DiscoveryRequest,DiscoveryResponseruntime.rsRuntimetraitconfig.rsClientConfigwatcher.rsResourceWatcher<T>,ResourceEvent<T>client.rsXdsClient,XdsClientBuilderIn case the reviewer is curious if the provided APIs could support the expected functionality of an xDS client, I have a working proof of concept that implements these APIs in my fork: https://github.com/YutaoMa/tonic/tree/yutaoma/xds-client-poc, it showcases the APIs working together with an implementation based on the
tonicstack (tonic,tokio,prost).