Configuration loading improvements (fixes #79) #81
Conversation
| @@ -100,7 +100,7 @@ pub trait Application: Default + Sized { | |||
| // Load configuration | |||
| let config = command | |||
| .config_path() | |||
| .map(|_| self.load_config(command)) | |||
| .map(|path| command.process_config(self.load_config(&path)?)) | |||
There was a problem hiding this comment.
What happens if load_config fails? Does the error bubble up to exit the program, or does it just not load the config (cf. #80)?
There was a problem hiding this comment.
It bubbles up here, and the application exits:
https://github.com/iqlusioninc/abscissa/blob/develop/src/application.rs#L58
If you'd like for a nonexistent config file to be ignored, the command's config_path() needs to check for the file's existence in advance, and if it's not there, return None
There was a problem hiding this comment.
Hmm, I'm confused because I thought from #80 (comment) that the intent was that config_path doesn't need to check. (Either way is fine, as long as it's clear one way or the other).
There was a problem hiding this comment.
It doesn't need to check if you're fine with the application exiting with an error if the config doesn't exist.
If you want an absent config to be ignored, config_path() needs to return None
- Have `command::EntryPoint` delegate its `Configurable` impl to the command it wraps. - Simplify configuration loading logic in `Application`. - Rename `config::MergeOptions` to `config::Override` and provide an example of how to use it in the boilerplate.
tony-iqlusion
changed the title
|
Removed WIP. I planned on trying to get a bit more into this commit to actually exercise and test the changes, but I think I'll do that in a followup which improves the configuration testing story. |
command::EntryPointdelegate itsConfigurableimpl to the command it wraps.Application.config::MergeOptionstoconfig::Overrideand provide an example of how to use it in the boilerplate.