Skip to content
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

ServerConf::from_yaml() & Co. should be implemented via a trait #232

Open
palant opened this issue May 7, 2024 · 1 comment
Open

ServerConf::from_yaml() & Co. should be implemented via a trait #232

palant opened this issue May 7, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@palant
Copy link

palant commented May 7, 2024

What is the problem your feature solves, or the need it fulfills?

Extending ServerConf with custom settings works along these lines:

#[derive(Debug, PartialEq, Eq, Deserialize)]
#[serde(default)]
pub struct MyAppConf {
    custom_setting: bool,

    #[serde(flatten)]
    server: ServerConf,
}

The problem is, ServerConf::from_yaml() and similar methods are not defined for this structure and have to be reimplemented.

Describe the solution you'd like

The methods from_yaml() and load_from_yaml() are generic and should be defined on a trait like FromYaml. These can then get a blanket implementation for anything implementing serde::Deserialize. This way any custom extension of ServerConf will have that functionality implemented automatically.

Method to_yaml() can also be a blanket implementation for anything implementing serde::Serialize.

This will make extending the default configuration easier. Another positive side-effect: serde_yaml will stay a dependency of Pingora and won’t have to be pulled in for the applications using it.

Describe alternatives you've considered

It isn’t too hard to reimplement this functionality for custom extensions of course, just unnecessary.

@andrewhavck andrewhavck added the enhancement New feature or request label May 9, 2024
@palant
Copy link
Author

palant commented May 9, 2024

For reference, so far I’m using my own implementation of this trait which can be found here: https://github.com/palant/pingora-utils/blob/main/module-utils/src/lib.rs#L89-L124.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants