(xds) decouple SubscriptionBase from SubscriptionCallbacks & rename#44047
(xds) decouple SubscriptionBase from SubscriptionCallbacks & rename#44047antoniovleonti wants to merge 7 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: antoniovleonti <leonti@google.com>
Signed-off-by: antoniovleonti <leonti@google.com>
Signed-off-by: antoniovleonti <leonti@google.com>
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the SubscriptionBase class into a new ResourceTypeHelper struct, promoting composition over inheritance for xDS clients. This change involves creating a new header and build target for ResourceTypeHelper and updating all existing xDS clients to use the new helper as a member. Review comments suggest marking the resource_type_helper_ member as const in various classes to enforce immutability, and also propose making resource_decoder_ private with a public getter in ResourceTypeHelper for better encapsulation.
Signed-off-by: antoniovleonti <leonti@google.com>
|
/assign @kyessenov |
Signed-off-by: antoniovleonti <leonti@google.com>
|
/assign @wbpcode |
Signed-off-by: antoniovleonti <leonti@google.com>
|
/retest |
|
/assign @krinkinmu |
|
/assign @adisuissa |
| const envoy::config::endpoint::v3::ClusterLoadAssignment& cluster_load_assignment_; | ||
| }; | ||
|
|
||
| const Config::ResourceTypeHelper<envoy::config::endpoint::v3::ClusterLoadAssignment> |
There was a problem hiding this comment.
Something to think about in the future: this is essentially the same resource_type_helper object that is created for all the EdsClusterImpl objects. In the future it may make more sense to create a single map between the type and the decoder, and use that.
|
/assign-from @envoyproxy/senior-maintainers |
|
@envoyproxy/senior-maintainers assignee is @wbpcode |
|
@wbpcode bump |
|
@wbpcode this seems to be waiting on your sign off |
|
/assign-from @envoyproxy/senior-maintainers |
|
@envoyproxy/senior-maintainers assignee is @wbpcode |
|
Sorry for the delay. Because this is a pure code improvement. It should be good to delay this until 1.38 to be released first? Slack me once 1.38 is out. |
6 participants
Commit Message: decouple SubscriptionBase from SubscriptionCallbacks & rename
Additional Description:
Decouple
SubscriptionBasefromSubscriptionCallbacksso thatSubscriptionBasecan be used with other types of callbacks (e.g.SingletonSubscriptionCallbacks).SubscriptionCallbacksfromSubscriptionBaseSubscriptionBasetoResourceTypeHelper.SubscriptionBase:ResourceTypeHelpermemberSubscriptionCallbacksRisk Level: low / none
Testing: no behavior change, all existing tests pass
Docs Changes: none
Release Notes: none
Context: #44237
Disclosure: an LLM was used to generate some of this change