You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
What is the problem your feature solves, or the need it fulfills?
Extending
ServerConf
with custom settings works along these lines: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()
andload_from_yaml()
are generic and should be defined on a trait likeFromYaml
. These can then get a blanket implementation for anything implementingserde::Deserialize
. This way any custom extension ofServerConf
will have that functionality implemented automatically.Method
to_yaml()
can also be a blanket implementation for anything implementingserde::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.
The text was updated successfully, but these errors were encountered: