wire up rouille to use config_manager (and delete old configs), then confirm everything still works
based on https://git.coopcloud.tech/PeachCloud/peach-workspace/issues/104
todo:
- wire up rouille to use config_manager (and delete old configs), then confirm everything still works
This PR is tested and working via an example I made in peach-lib/examples/config.rs.
With this config system it checks config keys in this order:
from environmental variables
from a configuration yaml file (by default located at /var/lib/peachcloud/config.yml but this path is itself configurable via setting an env variable PEACH_CONFIG_PATH)
from default values which are hard-coded in config_manager.rs
The primary interfaces for reading and saving config values are:
get_config_value does the appropriate lookup checking the three places.
save_config_value saves the given key, value to the configuration yaml.
Notes:
with this new system, config.yml is allowed to be incomplete. It just contains values which were explicitly saved. Any values which are not explicitly saved in config.yml will use hard-coded defaults
with this new system, we let configurations be unstructured strings by default. currently there are only two fields SSB_ADMIN_IDS (Vec) and DYN_ENABLED (bool) which were expected to be something other than a String. For these fields I've written custom getter and setter functions, which get and set these fields into the correct types. I feel OK with this, ad-hoc writing these if there are ever more configuration fields that need to be typed. Alternatively, we could add a stage to load_peach_config_from_disc and save_peach_config_to_disc which first converts the HashMap into a struct (where all fields are typed) before serializing or deserializing, but this would mean more code to keep up to date (mapping key names to struct fields), which might not really be worth it since the majority of fields are just String anyway
This PR is tested and working via an example I made in `peach-lib/examples/config.rs`.
With this config system it checks config keys in this order:
1. from environmental variables
2. from a configuration yaml file (by default located at `/var/lib/peachcloud/config.yml` but this path is itself configurable via setting an env variable PEACH_CONFIG_PATH)
3. from default values which are hard-coded in config_manager.rs
The primary interfaces for reading and saving config values are:
`get_config_value(key: &str) -> Result<String, PeachError>`
`save_config_value(key: &str, value: &str) -> Result<HashMap<String, String>, PeachError>`
get_config_value does the appropriate lookup checking the three places.
save_config_value saves the given key, value to the configuration yaml.
Notes:
- with this new system, config.yml is allowed to be incomplete. It just contains values which were explicitly saved. Any values which are not explicitly saved in config.yml will use hard-coded defaults
- with this new system, we let configurations be unstructured strings by default. currently there are only two fields SSB_ADMIN_IDS (Vec<String>) and DYN_ENABLED (bool) which were expected to be something other than a String. For these fields I've written custom getter and setter functions, which get and set these fields into the correct types. I feel OK with this, ad-hoc writing these if there are ever more configuration fields that need to be typed. Alternatively, we could add a stage to load_peach_config_from_disc and save_peach_config_to_disc which first converts the HashMap into a struct (where all fields are typed) before serializing or deserializing, but this would mean more code to keep up to date (mapping key names to struct fields), which might not really be worth it since the majority of fields are just String anyway
- before serializing the HashMap<String, String>, I convert it into a BTreeMap, so that it deterministically always serializes with the same alphabetical order of keys. Inspired by this SO post (https://stackoverflow.com/questions/42723065/how-to-sort-hashmap-keys-when-serializing-with-serde), although I'm not doing the exact same thing
notplants
marked the pull request as ready for review 2022-05-10 11:16:36 +00:00
notplants
changed title from Working on updating config_manager to Update config_manager to check from three sources of configuration2022-05-10 11:16:53 +00:00
Beautiful! I love the layered structure you've created; it feels very robust and the API for interacting with the config is clear and concise.
I just made one small suggestion with the dependency version and pointed out a tiny typo in a comment.
Beautiful! I love the layered structure you've created; it feels very robust and the API for interacting with the config is clear and concise.
I just made one small suggestion with the dependency version and pointed out a tiny typo in a comment.
Could we change this to 1.4? I got into a bad habit of pinning dependencies (specifying them all the way down to the patch version, ie. x.x.x) but I've been trying to undo that.
Pinning dependencies increases the likelihood that the compiler needs to compile several versions of the same crate. For example, if we have lazy_static = "1.4.0" and one of our dependencies uses lazy_static = "1.4.1" then both of those will need to be compiled and bundled in our binary.
Could we change this to `1.4`? I got into a bad habit of pinning dependencies (specifying them all the way down to the patch version, ie. `x.x.x`) but I've been trying to undo that.
Pinning dependencies increases the likelihood that the compiler needs to compile several versions of the same crate. For example, if we have `lazy_static = "1.4.0"` and one of our dependencies uses `lazy_static = "1.4.1"` then both of those will need to be compiled and bundled in our binary.
in other languages, I've assumed that pinning more precisely is preferrable, because then the library won't "shift under your feet" and change without you realizing (even though with minor number updates its supposed to not be breaking changes, any change is still a possible new bug),
but that makes sense in the rust case specifically it helps reduce compilation time and binary size
interesting!
in other languages, I've assumed that pinning more precisely is preferrable, because then the library won't "shift under your feet" and change without you realizing (even though with minor number updates its supposed to not be breaking changes, any change is still a possible new bug),
but that makes sense in the rust case specifically it helps reduce compilation time and binary size
I originally made this file just out of convenience of needing a way to test my code, but then thought it was kind of a neat pattern to keep little code examples around here.
yes I think so too!
I originally made this file just out of convenience of needing a way to test my code, but then thought it was kind of a neat pattern to keep little code examples around here.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
glyph
based on #104
todo:
This PR is tested and working via an example I made in
peach-lib/examples/config.rs.With this config system it checks config keys in this order:
/var/lib/peachcloud/config.ymlbut this path is itself configurable via setting an env variable PEACH_CONFIG_PATH)The primary interfaces for reading and saving config values are:
get_config_value(key: &str) -> Result<String, PeachError>save_config_value(key: &str, value: &str) -> Result<HashMap<String, String>, PeachError>get_config_value does the appropriate lookup checking the three places.
save_config_value saves the given key, value to the configuration yaml.
Notes:
Working on updating config_managerto Update config_manager to check from three sources of configurationBeautiful! I love the layered structure you've created; it feels very robust and the API for interacting with the config is clear and concise.
I just made one small suggestion with the dependency version and pointed out a tiny typo in a comment.
@@ -21,3 +21,4 @@ serde_json = "1.0"serde_yaml = "0.8"toml = "0.5"sha3 = "0.10"lazy_static = "1.4.0"Could we change this to
1.4? I got into a bad habit of pinning dependencies (specifying them all the way down to the patch version, ie.x.x.x) but I've been trying to undo that.Pinning dependencies increases the likelihood that the compiler needs to compile several versions of the same crate. For example, if we have
lazy_static = "1.4.0"and one of our dependencies useslazy_static = "1.4.1"then both of those will need to be compiled and bundled in our binary.interesting!
in other languages, I've assumed that pinning more precisely is preferrable, because then the library won't "shift under your feet" and change without you realizing (even though with minor number updates its supposed to not be breaking changes, any change is still a possible new bug),
but that makes sense in the rust case specifically it helps reduce compilation time and binary size
@@ -0,0 +1,11 @@use peach_lib::config_manager::{get_config_value, save_config_value};It's a neat pattern to include a code usage pattern as an example like this.
yes I think so too!
I originally made this file just out of convenience of needing a way to test my code, but then thought it was kind of a neat pattern to keep little code examples around here.
@@ -103,0 +159,4 @@// insert new key/valuepeach_config.insert(key.to_string(), value.to_string());// save hte modified hashmap to discTiny typo here on
save hte.