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

🐛 bring back loading provider flags from env vars #4863

Merged
merged 3 commits into from
Nov 25, 2024

Conversation

afiune
Copy link
Contributor

@afiune afiune commented Nov 15, 2024

The problem from my original PR #4847 to load provider flags from environment
variables was that I didn't consider the case where a provider defines a flag with
ConfigEntry = "-". For those cases, we do NOT bind the flag to the viper config
and therefore, we couldn't load these flags.

This PR fixes that issue and, for those flags that are not bind to the config, we will
continue to fetch the value directly from the flag itself, that is, from cobra.

When a provider defines a flag with `ConfigEntry = "-"`, then we do not
Bind the Flag to the `viper` config. For those flags, we will continue
to fetch the value directly from the flag, that is, from `cobra`.

Signed-off-by: Salim Afiune Maya <[email protected]>
@afiune afiune requested review from chris-rock and arlimus November 15, 2024 07:21
Copy link
Contributor

github-actions bot commented Nov 15, 2024

Test Results

3 145 tests  ±0   3 144 ✅ ±0   1m 36s ⏱️ +12s
  371 suites ±0       1 💤 ±0 
   28 files   ±0       0 ❌ ±0 

Results for commit fc3ada7. ± Comparison against base commit 42081a4.

♻️ This comment has been updated with latest results.

}
})

t.Run("command with flags set to not bind to config (ConfigEntry=\"-\")", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some weird spacing hin here

test/providers/os_test.go Outdated Show resolved Hide resolved
@afiune afiune merged commit 9cf3198 into main Nov 25, 2024
16 checks passed
@afiune afiune deleted the afiune/env-vars-are-back branch November 25, 2024 21:56
@github-actions github-actions bot locked and limited conversation to collaborators Nov 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants