mehcode
Environment variable names are converted to lower case
Environment variable names are converted to lower case internally. This is surprising undocumented behavior. We want to be able to override values from a configuration file with environment variables. It would be natural to use the same spelling (not counting the prefix), but this doesn't work if the configuration file keys are upper case. To make it work, the configuration file keys must be lower case.
config.toml
FOO="You can't override me!"
bar="You can override me just fine."
In the shell
APP_FOO="Why are you ignoring me?" APP_BAR="Yay! New bar." ./target/debug/app
I was using TOML for the config file and bash for the shell, if it matters.
Yes this happens because Environment variables are converted to lower case but this is not the case for the rest of the file formats supported by the crate.
For this kind of example to work, all the files in the folder /Format has to convert the key to lower case before it gets inserted into the internal Map of the Value.
I don't think this simple fix have any side effects. I am happy to submit a pull request.
Any update on this issue? I would also want the option not to convert env var into lowercase internally.
@matthiasbeyer I have wrote a patch to fix this, however, it turns out it's not as straight forward as I first thought. If we use the lowercase keys approach to fix this issue, then the following code where enums allow for uppercase fields, suddenly won't work for example.
config.toml
FOO = "I should be overridden"
main.rs
#[derive(Debug, Deserialize,PartialEq)]
enum TestEnum {
FOO(String)
}
let cfg= Config::builder()
.add_source(File::new("config.toml", FileFormat::Toml)).build().unwrap();
let foo: TestEnum = cfg.try_deserialize().unwrap();
// Error
the code won't work because all the keys inserted into the map are now lowercase, which means in the deserialization phase serde will fail to match the lowercase key foo with the enum field FOO.
but this will work great in the case of a struct since struct fields are usually lowercase.
So I think a better approach to fix this is to use the keys as they were initially set by the user of the crate in the environment variable internals .i.e, remove the lowercase. This will make the crate "case agnostic", then the users of the crate will have the freedom to set their keys as they wish, as well as the responsibility to make those keys match the data structure they want to deserialize into.
I have run some tests with this approach and it seems to work.
Let me know what you think about this, and or if you have any other concerns that I haven't taken into consideration.
Leaving the case as is makes sense to me. It's easy enough to override the Rust warnings about identifier case, i.e. #[allow(non_camel_case_types)]
@pictographer Yes, that could also be easily fixed by matching for uppercase in the deserialization phase for enums. It will be awesome if the same can be done for structs but that will require a manual struct deserialization. Also as a side note I am not sure why the crate limits enums to deserialize for one field only.
Are you familiar with the strum crate and the config crate? These two are awesome for unifying environment variables, enumeration constants, and configuration file values.
@pictographer you're referencing the config crate here? You realize that this is the codebase of the config crate? :laughing:
Leaving the case as is makes sense to me. It's easy enough to override the Rust warnings about identifier case
This.
I am eager to do things the right way in the next generation of this crate (see #321) and not put too much effort into the existing code IF it turns out that the next generation will actually be delivered at some point. I still invite you all, though, to propose fixes or changes to the existing implementation, of course!
@pictographer you're referencing the config crate here? You realize that this is the codebase of the config crate? laughing
Oops! I've gotta laugh. I wish my brain worked better.
@matthiasbeyer, thank you for the careful work! Nice to see this merged.