Don't let loaders override hardcoded configuration#185
Don't let loaders override hardcoded configuration#185
Conversation
pietern
changed the title
nfx
left a comment
nfx
left a comment
There was a problem hiding this comment.
Yes, this approach might work
databricks/config.go
Outdated
| // Read from ~/.databrickscfg if applicable. | ||
| KnownConfigLoader{}, | ||
| // Load from environment variables again (they have precedence). | ||
| ConfigAttributes.EnvironmentLoader(), |
pietern
changed the title
|
Without this PR:
With this PR:
Without checking for dups on 4 this is equivalent to the current approach and ambiguity persists. It is still possible to hardcode |
config/config.go
Outdated
| if len(loaders) == 0 { | ||
| loaders = []Loader{ | ||
| // Load from environment variables. | ||
| environmentVariableLoader{}, |
There was a problem hiding this comment.
giving it another thought: can we create a loader just for DATABRICKS_CONFIG_FILE / DATABRICKS_CONFIG_PROFILE and use it as the first one?... otherwise, in bricks cli - can we just specify the list of custom loaders to use only direct configuration?...
There was a problem hiding this comment.
Done.
What do you mean with:
otherwise, in bricks cli - can we just specify the list of custom loaders to use only direct configuration?...
?
| func createConfigurationFromLoaders(loaders []Loader) (*Config, error) { | ||
| if len(loaders) == 0 { | ||
| loaders = []Loader{ | ||
| // Load from environment variables. |
There was a problem hiding this comment.
could you also update the main readme file, because this behavior has to be documented on auth resolution. otherwise plenty of GH issues/ES tickets.
There was a problem hiding this comment.
The auth flow doesn't change.
|
i think this should be closed in favor of #306 |
2 participants
New approach of #170 (see comment).
The approach in this PR is simple: we load a zero-initialized configuration with 1) environment variables, 2) databrickscfg, 3) environment variables again, to make sure they have precedence over databrickscfgs.
This configuration is then merged into the target configuration, ignoring attributes that were already set.
The net result is that: