Handling missing environment variables cleaner for SWA#1825
Merged
seantleonard merged 11 commits intomainfrom Oct 20, 2023
Merged
Handling missing environment variables cleaner for SWA#1825seantleonard merged 11 commits intomainfrom
seantleonard merged 11 commits intomainfrom
Conversation
Test isn't implementing logic of #1820 yet, just replicating an existing test to ensure the test runner passes initially. Created in a new test class to reduce changes of merge conflicts, but extracted out shared logic from existing test class.
… in deserialization - New enum that controls the handling of the missing envrionment var - Default behaviour left unchanged to throw when encountered - New behaviour to ignore only implemented in the callsite of the config controller
aaronpowell
requested review from
Aniruddh25,
JelteF,
Mathos1432,
aaronburtle,
abhishekkumams,
ayush3797,
gledis69,
mbhaskar,
milismsft,
ravishetye,
seantleonard,
severussundar and
tarazou9
as code owners
Also updated asserts to test the value of the parsed config is as expected
Aniruddh25
requested changes
Oct 18, 2023
Collaborator
Aniruddh25
left a comment
Aniruddh25
left a comment
There was a problem hiding this comment.
Thanks for quick fix! Left some questions..
Left the updated code paths in so if the decision is changed we are perpared for it. Also left test in, but ignored, for future
Aniruddh25
reviewed
Oct 19, 2023
Aniruddh25
approved these changes
Oct 19, 2023
Collaborator
Aniruddh25
left a comment
Aniruddh25
left a comment
There was a problem hiding this comment.
LGTM! Thanks for addressing this.
seantleonard
requested changes
Oct 19, 2023
seantleonard
approved these changes
Oct 20, 2023
Contributor
seantleonard
left a comment
seantleonard
left a comment
There was a problem hiding this comment.
thanks for updating to ensure we have this functionality for Prod V1 config endpoint! and updating tests accordingly
2 tasks
Aniruddh25
pushed a commit
that referenced
this pull request
Oct 21, 2023
…1825) (#1834) Cherry pick #1825 to 0.9 ## Why make this change? - Fixes #1820 ## What is this change? - New test coverage for #1820 - Added a way to control error handling when env vars aren't set - default is left to throw, override to ignore ## How was this tested? - [x] Integration Tests - [ ] Unit Tests Co-authored-by: Aaron Powell <[email protected]>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Why make this change?
What is this change?
How was this tested?