Update config_manager to check from three sources of configuration #105

notplants · 2022-05-09T13:53:50Z

notplants commented

2022-05-09 13:53:50 +00:00

based on #104

todo:

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

notplants added 2 commits 2022-05-09 13:53:51 +00:00

29804b0dce Get CONFIG_PATH from env_variable

continuous-integration/drone/pr Build is failing Details

5b86f754f4 Working on refactor to use hashmaps

notplants added 1 commit 2022-05-10 10:59:57 +00:00

continuous-integration/drone/pr Build is failing Details

2540a77af1 Working 3-way configuration

notplants added 1 commit 2022-05-10 11:08:07 +00:00

continuous-integration/drone/pr Build is failing Details

600f9c58bf Use &str instead of String in save_config_value

notplants added 1 commit 2022-05-10 11:08:36 +00:00

continuous-integration/drone/pr Build is failing Details

8a6ad4ad61 Fix example

notplants commented

2022-05-10 11:15:11 +00:00

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(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) 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

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 changed title from ~~WIP: Working on updating config_manager~~ to Working on updating config_manager

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 configuration

2022-05-10 11:16:53 +00:00

notplants added 1 commit 2022-05-10 11:23:56 +00:00

continuous-integration/drone/pr Build is failing Details

3ae182caa9 Fix clippy warnings in peach-web

notplants added 1 commit 2022-05-10 11:26:02 +00:00

continuous-integration/drone/pr Build is passing Details

f4c1bc1169 Fix cargo fmt in peach-web

notplants requested review from glyph 2022-05-10 11:32:14 +00:00

glyph approved these changes 2022-05-10 12:17:37 +00:00

glyph left 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.

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.

peach-lib/Cargo.toml

						
				@ -21,3 +21,4 @@ serde_json = "1.0"

				serde_yaml = "0.8"

				toml = "0.5"

				sha3 = "0.10"

				lazy_static = "1.4.0"

glyph commented

2022-05-10 12:09:29 +00:00

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.

notplants commented

2022-05-11 08:31:14 +00:00

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

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

👍 1

peach-lib/examples/config.rs

						
				@ -0,0 +1,11 @@

				use peach_lib::config_manager::{get_config_value, save_config_value};

glyph commented

2022-05-10 12:10:46 +00:00

It's a neat pattern to include a code usage pattern as an example like this.

notplants commented

2022-05-11 08:35:43 +00:00

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.

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.

peach-lib/src/config_manager.rs Outdated

						
				@ -103,0 +159,4 @@

				    // insert new key/value

				    peach_config.insert(key.to_string(), value.to_string());

				    // save hte modified hashmap to disc