Ensures that boolean option flags are overridden by config.yml when no CLI flags are provided

[jack-cook-repo]

Previously, any values passed in a config.yml file for boolean flags (metabase_use_http, metabase_sync) were ignored.

This change will override any flags that are of type click.types.BoolParamType with values provided in a config.yml file, even if specified in a command line argument.

For example, when metabase_use_http is set to True in config.yml, running dbt-metabase models --metabase_https will still default to using HTTP.

Resolves #99

[gouline]

That's not an acceptable fix to the logic. Command-line flags should always override the config file, what you want is to use the config value when no command-line flag is passed.

So in your example, if metabase_use_http is set to True in config.yml, running dbt-metabase models --metabase_https should use HTTPS, but running dbt-metabase models should use HTTP.

[z3z1ma]

Hey @jack-cook-repo 👋

The precedence is:

1. CLI Flag
2. Env Var
3. config.yaml (consider these baseline defaults which can be overridden via either of the above methods)

Configuration is resolved in that order. 👆

We are injecting config ONLY if it 🗒️ : A) matches an existing CLI flag name in click context ctx B) no value was provided by the CLI flag or via env var

So we should really dump that context var ctx.params seen in the screenshot above and verify what the CLI flag is called in the context. Given what we see, the solution should be (hopefully) straightforward

I am wondering if metabase_https ends up in the context instead of the switch flags target var name of metabase_use_http

[jack-cook-repo]

Thanks both - I've pushed a change that checks, for boolean flags, whether ctx._parameter_source[f'{self.name}']._name_ == 'DEFAULT' (i.e., no CLI flag has been passed), then it checks to see whether a value was passed in the config file and overrides the default boolean value if so.

Testing No CLI flag, metabase_use_http: True specified in config.yml.

CLI flag that matches setting in config.yml

CLI flag that doesn't match setting in config.yml, and CLI flag takes precedence. The error is expected as it tries to connect to localhost via https.

This variable doesn't have an envvar set up so it just has preference of CLI flag > config.yml

[z3z1ma]

Looks like it needs to be formatted with black though. Make sure to run CI stuff locally @jack-cook-repo to verify everything. The makefile can make it easier. Otherwise good

[jack-cook-repo]

Thanks @z3z1ma - this is my first package contribution so I should have probably asked about the process!

I ran make which appears to have run the Makefile without any issue. ~- should I expected to have seen changes once black had run?~

Edit: I ran black lint separately and have pushed the updated file, should the makefile have done the same?

[gouline]

CI seems to be stalled but I'll merge it in. Sorry for the delay, haven't been looking at this repo.