Do not allow config attributes to be set multiple times#170
Do not allow config attributes to be set multiple times#170
Conversation
Doing so results in ambiguity; e.g. does it pick up the configuration from the profile in ~/.databrickscfg, the environment, or the constructed struct.
|
@nfx This needs discussion. The premise of this PR would actively deny the following because it's ambiguous. func TestConfig_AttributePrecedence(t *testing.T) {
configFixture{
host: "y",
env: map[string]string{
"DATABRICKS_HOST": "x",
"DATABRICKS_USERNAME": "x",
"DATABRICKS_PASSWORD": "x",
},
assertAuth: "basic",
assertHost: "https://y",
}.apply(t)
}The first commit only does this for overrides from the profile. I think we should also disallow overriding through environment variable if the struct already contains a host. |
|
The test fails because I want to discuss prior to changing the tests. |
| for _, attr := range alreadySet { | ||
| names = append(names, attr.Name) | ||
| } | ||
| return fmt.Errorf("attributes already set: %s", strings.Join(names, ", ")) |
There was a problem hiding this comment.
the failing tests are considered configuration contracts in the Terraform provider - e.g. "Public API". we cannot fail those.
I agree that configuration precedence can be tuned/changed, but only in a way that doesn't break the configuration contract.
Common sense is:
- Environment variable can override whatever was in the configuration file
- Directly configured value probably should not be overridable via env vars
Perhaps it could be best addressed via splitting ConfigAttributes into two different loaders
databricks-sdk-go/databricks/config.go
Lines 154 to 157 in 3e307bb
|
Superseded by #185. |
2 participants
Doing so results in ambiguity; e.g. does it pick up the configuration from the
profile in ~/.databrickscfg, the environment, or the constructed struct.