Skip to content

utility: convert typed_config to factory config#10418

Merged
htuch merged 2 commits intoenvoyproxy:masterfrom
yxue:test
Mar 18, 2020
Merged

utility: convert typed_config to factory config#10418
htuch merged 2 commits intoenvoyproxy:masterfrom
yxue:test

Conversation

@yxue
Copy link
Member

@yxue yxue commented Mar 17, 2020

Signed-off-by: Yan Xue yxyan@google.com

Description: convert typed_config to factory config
Risk Level: Low
Testing: Unit test
Docs Changes:
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: Yan Xue <yxyan@google.com>
@yxue
Copy link
Member Author

yxue commented Mar 17, 2020

@htuch I extracted the utility from #10369

Signed-off-by: Yan Xue <yxyan@google.com>
*/
template <class Factory>
static ProtobufTypes::MessagePtr
translateAnyToFactoryConfig(const ProtobufWkt::Any& typed_config,
Copy link
Member

Choose a reason for hiding this comment

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

Thanks, this looks clean. Are there some existing sites that we should refactor to use this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't do a thorough search but I guess existing proto should include the deprecated config field. Otherwise, this function should exist for converting the typed config to factory configuration.

We could replace all the translateToFactoryConfig with translateAnyToFactoryConfig because the config field is deprecated. It will break the compatibility.

@htuch htuch self-assigned this Mar 17, 2020
@htuch htuch added the waiting label Mar 17, 2020
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Thanks!

@htuch htuch merged commit 8704403 into envoyproxy:master Mar 18, 2020
@yxue yxue deleted the test branch March 18, 2020 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants