Fix EnvConfig to read from system environment variables #371
Conversation
Previously, when `override_env_vars` was not provided, the code passed
an empty hash (`override_env_vars || {}`) to the Rust bridge. This
caused the core library to use that empty hash as the env var source
instead of reading from the actual system environment.
The fix conditionally omits the argument when nil, allowing the Rust
bridge to treat it as optional and fall back to `std::env::var()`
for system environment variable lookups.
| ) | ||
| path, data = EnvConfig._source_to_path_and_data(config_source) | ||
|
|
||
| raw_profile = Internal::Bridge::EnvConfig.load_client_connect_config( |
There was a problem hiding this comment.
Can you just leave the existing code and just remove || {}?
There was a problem hiding this comment.
I initially did this, I thought Rust's Option<HashMap> would allow for nil values from Ruby, but scan_args tries to convert nil to HashMap.
There was a problem hiding this comment.
You could just spread the optional arg: *([override_env_vars] unless override_env_vars.nil?)
There was a problem hiding this comment.
I would rather not splat at this point. I think you should make every argument required from the Rust side. It's internal code, we should not add optional fields on ourselves. Can move to structs if needed since param list is growing large.
There was a problem hiding this comment.
Changed from splatting to explicit conditional. Making the change Rust side is prudent but I'd prefer not to delay the fix.
| end | ||
| end | ||
|
|
||
| def with_system_env_vars(env_vars) |
There was a problem hiding this comment.
Need to add a decl to the .rbs:
def with_system_env_vars: [T] (Hash[String, String] env_vars) { () -> T } -> T
Previously, when
override_env_varswas not provided, the code passed an empty hash (override_env_vars || {}) to the Rust bridge. This caused the core library to use that empty hash as the env var source instead of reading from the actual system environment.The fix conditionally omits the argument when nil, allowing the Rust bridge to treat it as optional and fall back to
std::env::var()for system environment variable lookups.Why?
Bug fix
Added test