feat(jans-cedarling): Load bootstrap properties from environment variables #10692
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…alize` that allows to deserialize struct from string Signed-off-by: Oleh Bohzok <[email protected]>
Signed-off-by: Oleh Bohzok <[email protected]>
Signed-off-by: Oleh Bohzok <[email protected]>
…prove tests Signed-off-by: Oleh Bohzok <[email protected]>
Signed-off-by: Oleh Bohzok <[email protected]>
Signed-off-by: Oleh Bohzok <[email protected]>
Signed-off-by: Oleh Bohzok <[email protected]>
Signed-off-by: Oleh Bohzok <[email protected]>
Signed-off-by: Oleh Bohzok <[email protected]>
mo-auto
added
area-documentation
olehbozhok
removed
area-documentation
olehbozhok
changed the title
mo-auto
added
area-documentation
Signed-off-by: Oleh Bohzok <[email protected]>
Signed-off-by: Oleh Bohzok <[email protected]>
olehbozhok
force-pushed
the
jans-cedaling-issue-10648
branch
from
c046d85 to
b2df993
Compare
rmarinn
reviewed
Jan 18, 2025
Comment on lines
+133
to
+136
| // OR from environment variables | ||
| let config = | ||
| BootstrapConfig::from_raw_config_and_env(None).unwrap(); | ||
|
|
There was a problem hiding this comment.
So if None is passed, it will just used defaults... should we also provide a function where the behavior is like passing in None?
Suggested change
| // OR from environment variables | |
| let config = | |
| BootstrapConfig::from_raw_config_and_env(None).unwrap(); | |
| // Load the bootstrap config from the environment variables. Properties that are not defined will be assigned a default value. | |
| let config = BootstrapConfig::from_env().unwrap(); | |
| // Load the bootstrap config from the environment variables and a given config. | |
| let config = BootstrapConfig::from_raw_config_and_env(None).unwrap(); |
Comment on lines
+468
to
+471
| /// Construct `BootstrapConfig` from environment variables and `BootstrapConfigRaw` config | ||
| // | ||
| // Simple implementation that map input structure to JSON map | ||
| // and map environment variables with prefix `CEDARLING_` to JSON map. And merge it. |
There was a problem hiding this comment.
Also document which config source has a higher priority here.
| @@ -428,7 +453,41 @@ pub struct ParseFeatureToggleError { | |||
| value: String, | |||
| } | |||
|
|
|||
| /// Get environment variables related to `Cedarling` | |||
| #[cfg(not(target_arch = "wasm32"))] | |||
| fn cedarling_env_vars() -> HashMap<String, serde_json::Value> { | |||
There was a problem hiding this comment.
load_cedarling_env_vars or get_cedarling_env_vars might be a better name here
Comment on lines
+25
to
+27
| /// Attempts to deserialize a value, falling back to JSON parsing if the value is a string. | ||
| /// Returns the deserialized value or the original error if both attempts fail. | ||
| pub fn fallback_deserialize<'de, D, T>(deserializer: D) -> Result<T, D::Error> |
There was a problem hiding this comment.
a better name might be deserialize_or_parse_string_as_json
Suggested change
| /// Attempts to deserialize a value, falling back to JSON parsing if the value is a string. | |
| /// Returns the deserialized value or the original error if both attempts fail. | |
| pub fn fallback_deserialize<'de, D, T>(deserializer: D) -> Result<T, D::Error> | |
| /// Attempts to deserialize a value, falling back to JSON parsing if the value is a string. | |
| /// Returns the deserialized value or the original error if both attempts fail. | |
| pub fn deserialize_or_parse_string_as_json<'de, D, T>(deserializer: D) -> Result<T, D::Error> |
Comment on lines
+9
to
+10
| /// Helper function to convert Python-style list strings to JSON format | ||
| fn to_json(s: &str) -> Option<Value> { |
There was a problem hiding this comment.
a better name might be parse_python_list_to_json
Suggested change
| /// Helper function to convert Python-style list strings to JSON format | |
| fn to_json(s: &str) -> Option<Value> { | |
| /// Helper function to convert Python-style list strings to JSON format | |
| fn parse_python_list_to_json(s: &str) -> Option<Value> { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prepare
Description
Implemented load
BootstrapConfigfrom environment variablesTarget issue
link
closes #10648
Implementation Details
BootstrapConfigfrom environment variablesTest and Document the changes
Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with
docs:to indicate documentation changes or if the below checklist is not selected.