Align bridge env config loading behavior#767
Conversation
jmaeagle99
left a comment
jmaeagle99
left a comment
There was a problem hiding this comment.
Approving, but please update the changelog at the very least.
| try | ||
| { | ||
| var envVarsRef = options.OverrideEnvVars?.Count > 0 | ||
| var envVarsRef = options.OverrideEnvVars != null |
There was a problem hiding this comment.
This is technically a breaking change, even if the comments on the OverrideEnvVars property were correct on the intended behavior. This very minor breaking change needs to be docuemnted in the changelog and the PR title updated with a 💥.
| try | ||
| { | ||
| var envVarsRef = options.OverrideEnvVars?.Count > 0 | ||
| var envVarsRef = options.OverrideEnvVars != null |
There was a problem hiding this comment.
And since this is changing a behavior, a small test would be warranted, if possible.
There was a problem hiding this comment.
While the vast majority of the env application is done in core, the marshaling decision is owned by the .NET side. Making a test for this might prove difficult because calling Environment.SetEnvironmentVariable will not have observable result in core; the environment block is loaded by the .NET runtime once and mutated in memory in the .NET runtime without being reflected into the native process environment.
2 participants
What was changed
Pass empty
OverrideEnvVarsthrough to bridge environment config loading instead of treating an empty override dictionary as unset.Why?
This keeps bridge config-loading behavior aligned with other SDKs and lets an explicit empty override suppress the process environment. This PR was split out from
ea/aws-lambdaso theEnvConfig.cschange can be reviewed independently.Checklist
Closes N/A
How was this tested:
Not run; branch split only.
Any docs updates needed?
No.