-
Notifications
You must be signed in to change notification settings - Fork 98
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
Update proxy configuration format #144
Conversation
Update proxy configuration format Update tests and docs Merge the server/client configuration example Fixes #130
7f43a99
to
597e723
Compare
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.
Only blocking question I have is around ConnectionConfig
- otherwise, looks really good!
I went back and forth on Source
as a name - but I think it's actually the best name we could come up for that enum 👍
} | ||
|
||
#[derive(Debug, Deserialize, Serialize)] | ||
pub struct AdminAddress { |
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.
Nice.
@@ -36,6 +36,7 @@ pub mod external_doc_tests { | |||
// it is only available using a nightly compiler. | |||
// To run them locally run e.g `cargo +nightly test --doc` | |||
#![doc(include = "../docs/extensions/filters/filters.md")] | |||
#![doc(include = "../docs/extensions/filters/load_balancer.md")] |
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.
Nice catch!
connection: &'a ConnectionConfig, | ||
config: Option<&'a serde_yaml::Value>, | ||
) -> CreateFilterArgs<'a> { | ||
pub fn new(config: Option<&serde_yaml::Value>) -> CreateFilterArgs { |
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.
The idea behind passing through the ConnectionConfig
was so that Filters would be able to know if they are in client or server mode.
Is it worth adding that back in here (pass in the ProxyMode)? Or should we do it with whatever PR actually needs that information?
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.
Ah, yes it rings a bell now I remember we had some feature in mind that wanted it. We can include it back in the PR that needs it
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.
🚢
Update proxy configuration format
Update tests and docs
Merge the server/client configuration example
Fixes #130
Currently contains #140 so will keep in draft until that's merged