-
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
Add Validation to Config #68
Conversation
We needed validation that endpoint names are unique, so implemented a validation function for config. This had a few ripple on effects: - impl block for Config, so made sense to pull `from_reader` into that as a static method. - Needed concrete Error types (impacts #67) for validation and other potential error conditions. `from_reader` will now call validate() to ensure that the struct is valid before completing. `validate` was also left as public, since Config's can be manually created before passing them into `Server.run()`
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.
This looks good! I think config validation is something filters will depend on a lot too down the line!
Updated! |
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.
LGTM!
src/config.rs
Outdated
impl Config { | ||
/// from_reader returns a config from a given Reader | ||
pub fn from_reader<R: io::Read>(input: R) -> Result<Config, serde_yaml::Error> { | ||
let config: Config = serde_yaml::from_reader(input)?; |
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.
rust lets us do
pub fn from_reader<R: io::Read>(input: R) -> Result<Config, serde_yaml::Error> {
serde_yaml::from_reader(input)
}
if we wanted to avoid specify the type or rewrapping the value
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.
Oh yeah, why isn't this just one line. I'll clean this up before merging.
683093c
to
9b579f7
Compare
We needed validation that endpoint names are unique, so implemented a validation function for config. This had a few ripple on effects:
from_reader
into that as a static method.from_reader
will now call validate() to ensure that the struct is valid before completing.validate
was also left as public, since Config's can be manually created before passing them intoServer.run()