Config file envvar overrides #20
Merged
Conversation
This file contains hidden or 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
5t0n3
reviewed
Nov 6, 2024
| where | ||
| E: de::Error, | ||
| { | ||
| Ok(FromStr::from_str(value).unwrap()) |
Member
There was a problem hiding this comment.
unwrap here probably isn't ideal, I'd prefer returning a de::Error variant (maybe with de::Error::invalid_value?)
Contributor
Author
There was a problem hiding this comment.
value here is always going to be a &str, so AFAICT there's no chance it will ever fail here. I stole this code from Serde's documentation examples so I'm also not really sure why this is doing the from_str dance on an existing &str to begin with.
cacama-valvata
approved these changes
Nov 6, 2024
cacama-valvata
left a comment
cacama-valvata
left a comment
There was a problem hiding this comment.
Reviewed synchronously 11/05/24 -- approved
serde-yaml is deprecated and not maintained, serde-yml is the new fork Signed-off-by: Robert Detjens <github@detjens.dev>
This file is autogenerated and we (users) don't care about the specifics of what has changed in it, only that it has. Signed-off-by: Robert Detjens <github@detjens.dev>
Signed-off-by: Robert Detjens <github@detjens.dev>
Signed-off-by: Robert Detjens <github@detjens.dev>
detjensrobert
force-pushed
the
dr/config-refactor
branch
from
2945c1d to
ab6d6b8
Compare
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.
Switch to building global and challenge configs with Figment instead of directly with Serde. Figment can merge multiple sources together before deserializing, including from environment variables.
This gets us envvar overrides for basically free. Any config in
rcds.yamlcan be set from envvars --BEAVERCDS_<path>. Fields that are overridden from envvars can be omitted from the config file, since parsing happens after the merge. This allows users to not need to specify a placeholder value that's immediately replaced.Previously in #15 we agreed on
from_env:subkeys, but that would need us to do all the parsing and substitution ourselves. With this approach this library does these overrides for free, and also gets us very good test capabilities for overrides and parsing and so on.examples:
Fixes #15.
Supersedes #18.