-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Converting mongo proxy to v2 api #1952
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
Changes from all commits
49bac68
85d1d3a
40ebeb7
943804a
17f45ef
a6e6b0e
a43e50c
097e4d4
33ecf79
8d66953
b434fa0
7f6f4fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,8 @@ | |
|
|
||
| #include "common/config/well_known_names.h" | ||
|
|
||
| #include "api/filter/network/mongo_proxy.pb.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Server { | ||
| namespace Configuration { | ||
|
|
@@ -18,8 +20,15 @@ class MongoProxyFilterConfigFactory : public NamedNetworkFilterConfigFactory { | |
| // NamedNetworkFilterConfigFactory | ||
| NetworkFilterFactoryCb createFilterFactory(const Json::Object& config, | ||
| FactoryContext& context) override; | ||
| NetworkFilterFactoryCb createFilterFactoryFromProto(const Protobuf::Message& config, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You will also need to provider the factory https://github.com/envoyproxy/envoy/blob/master/include/envoy/server/filter_config.h#L173.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make sure we have tests that cover both the legacy and factory config flows also when you fix this. Thank you!
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have not touched any existing tests other than fixing up the class names. And the existing tests use JSON config for testing. i.e. they exercise the conversion and tests pass.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right my point is can you please make sure that the native proto loading path for v2 config is covered, and not just the conversion path. You may need to add new tests. I think this is important as we add the rest of the filters. |
||
| FactoryContext& context) override; | ||
|
|
||
| std::string name() override { return Config::NetworkFilterNames::get().MONGO_PROXY; } | ||
|
|
||
| private: | ||
| NetworkFilterFactoryCb | ||
| createMongoProxyFactory(const envoy::api::v2::filter::network::MongoProxy& mongo_proxy, | ||
| FactoryContext& context); | ||
| }; | ||
|
|
||
| } // namespace Configuration | ||
|
|
||
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.
Nit: probably should define
JSON_UTIL_SET_DURATIONin terms of this macro now.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.
can I do this in next PR for redis?
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.
Yes that's fine. Please do in the next PR.