Skip to content

CDS: Factor out some functionality of CDS into a helper class#15806

Merged
mattklein123 merged 1 commit intoenvoyproxy:mainfrom
kinvolk:krnowak/cds-api-helper
Apr 1, 2021
Merged

CDS: Factor out some functionality of CDS into a helper class#15806
mattklein123 merged 1 commit intoenvoyproxy:mainfrom
kinvolk:krnowak/cds-api-helper

Conversation

@krnowak
Copy link
Copy Markdown
Contributor

@krnowak krnowak commented Apr 1, 2021

Commit Message:
We plan to add an OdCdsApiImpl class next to CdsApiImpl. It's config
updated callback functionality would be similar to CdsApiImpl's, but
with some extra code. To avoid the duplication of code, we move it to
a helper class that will be used by both OdCdsApiImpl and CdsApiImpl.

Additional Description:
This is split out from #15523 to ease the review. It has no functional changes.

Risk Level:
Low.

Testing:
Tested as a part of #15523.

Docs Changes:
Added API docs for the newly added CdsApiHelper class.

Release Notes:
None needed.

Platform Specific Features:
None.

We plan to add an OdCdsApiImpl class next to CdsApiImpl. It's config
updated callback functionality would be similar to CdsApiImpl's, but
with some extra code. To avoid the duplication of code, we move it to
a helper class that will be used by both OdCdsApiImpl and CdsApiImpl.

Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
@repokitteh-read-only
Copy link
Copy Markdown

Hi @krnowak, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #15806 was opened by krnowak.

see: more, trace.

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you for splitting this out!


private:
ClusterManager& cm_;
std::string name_;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: const (just go ahead and do this in your next change)

@mattklein123 mattklein123 merged commit 63f08c4 into envoyproxy:main Apr 1, 2021
@krnowak krnowak deleted the krnowak/cds-api-helper branch April 1, 2021 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants