Conversation
|
|
||
| /// ClientConfig represents a client config file. | ||
| #[derive(Debug, Clone, PartialEq, Default)] | ||
| pub struct ClientConfig { |
There was a problem hiding this comment.
I wonder for pure Rust users if we want to support a builder on this too, but I'm fine without it
Sushisource
left a comment
Sushisource
left a comment
There was a problem hiding this comment.
Looking pretty good, can tighten up some types.
|
|
||
| /// Options for loading a client configuration profile | ||
| #[derive(Debug, Default)] | ||
| pub struct LoadClientConfigProfileOptions<'a> { |
There was a problem hiding this comment.
Imo for both this and the one before, don't include env_lookup by ref here inside the struct, I would just pass it separately into the function that accepts these
There was a problem hiding this comment.
And since we are using this in lang that we probably won't have callbacks before, I'd be ok just accepting a hash map of string-string here to override env instead of a trait.
There was a problem hiding this comment.
I've opted for a hash map of string-string. In either case lang has to implement something to get env vars, I think passing the result of that implementation (i.e. the hash map) is simpler bridge code than passing the trait implementation itself.
| /// Path to client mTLS certificate. Mutually exclusive with ClientCertData. | ||
| pub client_cert_path: Option<String>, | ||
|
|
||
| /// PEM bytes for client mTLS certificate. Mutually exclusive with ClientCertPath. | ||
| pub client_cert_data: Option<Vec<u8>>, |
There was a problem hiding this comment.
This should just be a little enum that allows you to pick one of the other, and is non-optional. Embrace types that make bad states unrepresentable.
There was a problem hiding this comment.
done
In place, I've added validation that a user cannot specify both path and data in their TOML + env vars, apply_data_source_env_var.
You can overwrite a TOML field with the same env var (i.e. env var path -> TOML path), but supplying some combination of:
- TOML path & TOML data
- TOML path & env data (or vice versa)
- env path & env data
is an invalid configuration.
Same logic applies to other mutually exclusive fields.
…ding both, read env vars from hashmap instead of trait env lookup
3 participants
What was changed
Added
envconfig.rs, a Rust port of the Goenvconfigmodule. This is intended to be used by lang-SDKs, which will effectively have a wrapper on top of this, along with some additional functionality for environment configs to interoperate with SDK-specific client options.Why?
Customer demand, useful utility
Part of the work for [Feature Request] Environment configuration features#441
How was this tested:
Ported over the relevant tests from the Go module, along with a couple additional tests.
Any docs updates needed?
No ?