From ebf910640f5bd142884f157a93c36645233d4573 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Thu, 25 Apr 2024 17:37:26 -0700 Subject: [PATCH] Update `warn_error_options` in CLI args to support `error` and `warn` options Setting `error` is the same as setting `include`, but only one can be specified. Setting `warn` is the same as setting `exclude`, but only one can be specified. --- core/dbt/cli/option_types.py | 28 +++++++++++++-- .../configs/test_warn_error_options.py | 36 ++++++++++++++++++- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/core/dbt/cli/option_types.py b/core/dbt/cli/option_types.py index 71291177c0c..b95fb3b9d64 100644 --- a/core/dbt/cli/option_types.py +++ b/core/dbt/cli/option_types.py @@ -3,9 +3,9 @@ from dbt.config.utils import parse_cli_yaml_string from dbt.events import ALL_EVENT_NAMES from dbt.exceptions import ValidationError, OptionNotYamlDictError -from dbt_common.exceptions import DbtValidationError - +from dbt_common.exceptions import DbtConfigError, DbtValidationError from dbt_common.helper_types import WarnErrorOptions +from typing import Any, Dict class YAML(ParamType): @@ -44,6 +44,28 @@ def convert(self, value, param, ctx): return {"name": value, "version": None} +def exclusive_primary_alt_value_setting( + dictionary: Dict[str, Any], primary: str, alt: str +) -> None: + """Munges in place under the primary the options for the primary and alt values + + Sometimes we allow setting something via TWO keys, but not at the same time. If both the primary + key and alt key have values, an error gets raised. If the alt key has values, then we update + the dictionary to ensure the primary key contains the values. If neither are set, nothing happens. + """ + + primary_options = dictionary.get(primary) + alt_options = dictionary.get(alt) + + if primary_options and alt_options: + raise DbtConfigError( + f"Only `{alt}` or `{primary}` can be specified in `warn_error_options`, not both" + ) + + if alt_options: + dictionary[primary] = alt_options + + class WarnErrorOptionsType(YAML): """The Click WarnErrorOptions type. Converts YAML strings into objects.""" @@ -52,6 +74,8 @@ class WarnErrorOptionsType(YAML): def convert(self, value, param, ctx): # this function is being used by param in click include_exclude = super().convert(value, param, ctx) + exclusive_primary_alt_value_setting(include_exclude, "include", "error") + exclusive_primary_alt_value_setting(include_exclude, "exclude", "warn") return WarnErrorOptions( include=include_exclude.get("include", []), diff --git a/tests/functional/configs/test_warn_error_options.py b/tests/functional/configs/test_warn_error_options.py index 24051979de4..3ab15974f72 100644 --- a/tests/functional/configs/test_warn_error_options.py +++ b/tests/functional/configs/test_warn_error_options.py @@ -71,6 +71,14 @@ def test_can_raise_warning_to_error( result = runner.invoke(["run", "--warn-error-options", "{'include': 'all'}"]) assert_deprecation_error(result) + catcher.flush() + result = runner.invoke(["run", "--warn-error-options", "{'error': ['DeprecatedModel']}"]) + assert_deprecation_error(result) + + catcher.flush() + result = runner.invoke(["run", "--warn-error-options", "{'error': 'all'}"]) + assert_deprecation_error(result) + def test_can_exclude_specific_event( self, project, catcher: EventCatcher, runner: dbtRunner ) -> None: @@ -79,10 +87,36 @@ def test_can_exclude_specific_event( catcher.flush() result = runner.invoke( - ["run", "--warn-error-options", "{'include': 'all', exclude: ['DeprecatedModel']}"] + ["run", "--warn-error-options", "{'include': 'all', 'exclude': ['DeprecatedModel']}"] ) assert_deprecation_warning(result, catcher) + catcher.flush() + result = runner.invoke( + ["run", "--warn-error-options", "{'include': 'all', 'warn': ['DeprecatedModel']}"] + ) + assert_deprecation_warning(result, catcher) + + def test_cant_set_both_include_and_error(self, project, runner: dbtRunner) -> None: + result = runner.invoke( + ["run", "--warn-error-options", "{'include': 'all', 'error': 'all'}"] + ) + assert not result.success + assert result.exception is not None + assert "Only `error` or `include` can be specified" in str(result.exception) + + def test_cant_set_both_exclude_and_warn(self, project, runner: dbtRunner) -> None: + result = runner.invoke( + [ + "run", + "--warn-error-options", + "{'include': 'all', 'exclude': ['DeprecatedModel'], 'warn': ['DeprecatedModel']}", + ] + ) + assert not result.success + assert result.exception is not None + assert "Only `warn` or `exclude` can be specified" in str(result.exception) + class TestWarnErrorOptionsFromProject: @pytest.fixture(scope="function")