Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for env var precedence and to for File provider required field #131

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

autodidaddict
Copy link

  • Fix so the lack of an environment variable subsequent in a pipeline won't cause it to wipe previously established values
  • Fix so that the File provider now honors the required field (previously ignored, non-required but missing files would cause a crash)

@autodidaddict
Copy link
Author

@keathley just checking in to see if this is something that might get looked at?

@bsedat
Copy link

bsedat commented Mar 17, 2022

As a note for anyone else, an equivalent patch can also be made to the File provider to allow it to supersede env var bindings when an overridden attribute is missing from the file.

@brooksmtownsend
Copy link

Hey @keathley / @bgmarx , is this something that's being considered?

@mhanberg
Copy link
Member

mhanberg commented Jun 1, 2022

@autodidaddict do you mind adding a unit test for these two cases? And also maybe an example scenario in the PR description? I think I understand the precedence problem you're describing but it would be great to have an actual example to see.

Thanks!

@autodidaddict
Copy link
Author

just checking in since it's been a while and I didn't remember your comment from June. we've been using this fork in production for right around a year now. Do you still want the unit tests, etc @mhanberg ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants