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

[CT-1798] Add mutually exclusive handling for warn_error_options and warn_error params in Click CLI #6579

Closed
3 tasks done
Tracked by #5527 ...
MichelleArk opened this issue Jan 11, 2023 · 2 comments
Closed
3 tasks done
Tracked by #5527 ...
Assignees
Labels
cli python_api Issues related to dbtRunner Python entry point

Comments

@MichelleArk
Copy link
Contributor

Is this your first time submitting a feature request?

  • I have read the expectations for open source contributors
  • I have searched the existing issues, and I could not find an existing issue for this feature
  • I am requesting a straightforward extension of existing dbt functionality, rather than a Big Idea better suited to a discussion

Describe the feature

The --warn-error-options flag was shipped as part of dbt-core 1.4 via #6520. --warn-error-options was added as an option in the new click CLI params, but the mutually exclusive behaviour has not yet been implemented in the click CLI - --warn-error and --warn-error-options should be mutually exclusive - whether they are specified through the CLI, user_config, or env vars.

Note: Look for opportunities to improve the logging the mutually exclusive options are specified from various sources (env var vs cli vs user config): #6520 (comment)

Describe alternatives you've considered

No response

Who will this benefit?

No response

Are you interested in contributing this feature?

No response

Anything else?

No response

@MichelleArk MichelleArk added the python_api Issues related to dbtRunner Python entry point label Jan 11, 2023
@github-actions github-actions bot changed the title Add mutually exclusive handling for warn_error_options and warn_error params in Click CLI [CT-1798] Add mutually exclusive handling for warn_error_options and warn_error params in Click CLI Jan 11, 2023
@iknox-fa
Copy link
Contributor

Potential entry point to param validation:
https://click.palletsprojects.com/en/8.1.x/api/#parameters
(see the section on param callbacks)

@MichelleArk
Copy link
Contributor Author

MichelleArk commented Jan 30, 2023

Looked into the callback approach suggest above, but don't think it's the appropriate level of handling for this functionality for a couple reasons:

  1. Expected callback signature is Callable[[Context, Parameter, Any], Any], and is called after processing an individual parameter value. The callback may or may not access to the set values of other parameters at this point depending on the parsing order, which feels fairly brittle and could lead to unexpected behaviour depending on the param parsing order.
  2. The callback is called before the flags module is initialized, meaning it doesn't have awareness of user configs. Given that mutually exclusive options can be set from multiple sources (user configs, env vars, cli), it would require a more complicated refactor of how we incorporate user configs to flags to provide the callback with the context it needs to accurately determine whether two mutually exclusive flags have been set together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli python_api Issues related to dbtRunner Python entry point
Projects
None yet
Development

No branches or pull requests

3 participants