cloudflare / cloudflare/pingora

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

Open
#232 1 comment 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Rust
Stars
27.4k
Forks
1.7k
Avg merge
6h 22m
Merged PRs (30d)
3

Description

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

Extending `ServerConf` with custom settings works along these lines:

```rust
#[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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.