From 2a8b0d6259d27b0615cef260ab67052c345a33e3 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Thu, 18 Jul 2024 10:37:28 -0700 Subject: [PATCH] Fix warn error options being invalid when warn or error set to none (#10453) (#10462) * Fix exclusive_primary_alt_value_setting to set warn_error_options correctly * Add test * Changie * Fix unit test * Replace conversion method * Refactor normalize_warn_error_options Co-authored-by: Gerda Shank --- .../unreleased/Fixes-20240716-171427.yaml | 6 +++ core/dbt/cli/option_types.py | 9 +---- core/dbt/config/project.py | 17 +++------ core/dbt/config/utils.py | 17 ++++++++- .../configs/test_warn_error_options.py | 29 ++++++++++++++- tests/unit/config/test_utils.py | 37 ++++++++++++++++++- 6 files changed, 92 insertions(+), 23 deletions(-) create mode 100644 .changes/unreleased/Fixes-20240716-171427.yaml diff --git a/.changes/unreleased/Fixes-20240716-171427.yaml b/.changes/unreleased/Fixes-20240716-171427.yaml new file mode 100644 index 00000000000..5d8db74a448 --- /dev/null +++ b/.changes/unreleased/Fixes-20240716-171427.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Fix error constructing warn_error_options +time: 2024-07-16T17:14:27.837171-04:00 +custom: + Author: gshank + Issue: "10452" diff --git a/core/dbt/cli/option_types.py b/core/dbt/cli/option_types.py index 544b09bee2a..d02cdc47ac3 100644 --- a/core/dbt/cli/option_types.py +++ b/core/dbt/cli/option_types.py @@ -1,6 +1,6 @@ from click import ParamType, Choice -from dbt.config.utils import parse_cli_yaml_string, exclusive_primary_alt_value_setting +from dbt.config.utils import normalize_warn_error_options, parse_cli_yaml_string from dbt.events import ALL_EVENT_NAMES from dbt.exceptions import ValidationError, OptionNotYamlDictError from dbt_common.exceptions import DbtValidationError @@ -51,12 +51,7 @@ 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", "warn_error_options" - ) - exclusive_primary_alt_value_setting( - include_exclude, "exclude", "warn", "warn_error_options" - ) + normalize_warn_error_options(include_exclude) return WarnErrorOptions( include=include_exclude.get("include", []), diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index 52b20e5179d..deaa99df343 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -16,6 +16,10 @@ from dbt.flags import get_flags from dbt import deprecations +from dbt.adapters.contracts.connection import QueryComment +from dbt.clients.yaml_helper import load_yaml_text +from dbt.config.selectors import SelectorDict +from dbt.config.utils import normalize_warn_error_options from dbt.constants import ( DEPENDENCIES_FILE_NAME, PACKAGES_FILE_NAME, @@ -23,8 +27,6 @@ DBT_PROJECT_FILE_NAME, ) from dbt_common.clients.system import path_exists, load_file_contents -from dbt.clients.yaml_helper import load_yaml_text -from dbt.adapters.contracts.connection import QueryComment from dbt.exceptions import ( DbtProjectError, DbtExclusivePropertyUseError, @@ -39,8 +41,6 @@ from dbt.version import get_installed_version from dbt.utils import MultiDict, md5, coerce_dict_str from dbt.node_types import NodeType -from dbt.config.selectors import SelectorDict -from dbt.config.utils import exclusive_primary_alt_value_setting from dbt.contracts.project import ( Project as ProjectContract, SemverString, @@ -839,13 +839,8 @@ def read_project_flags(project_dir: str, profiles_dir: str) -> ProjectFlags: if project_flags is not None: # handle collapsing `include` and `error` as well as collapsing `exclude` and `warn` # for warn_error_options - warn_error_options = project_flags.get("warn_error_options") - exclusive_primary_alt_value_setting( - warn_error_options, "include", "error", "warn_error_options" - ) - exclusive_primary_alt_value_setting( - warn_error_options, "exclude", "warn", "warn_error_options" - ) + warn_error_options = project_flags.get("warn_error_options", {}) + normalize_warn_error_options(warn_error_options) ProjectFlags.validate(project_flags) return ProjectFlags.from_dict(project_flags) diff --git a/core/dbt/config/utils.py b/core/dbt/config/utils.py index f94e5dce0ad..1d0e8e935ad 100644 --- a/core/dbt/config/utils.py +++ b/core/dbt/config/utils.py @@ -50,5 +50,18 @@ def exclusive_primary_alt_value_setting( f"Only `{alt}` or `{primary}` can be specified{where}, not both" ) - if alt_options: - dictionary[primary] = alt_options + if alt in dictionary: + alt_value = dictionary.pop(alt) + dictionary[primary] = alt_value + + +def normalize_warn_error_options(warn_error_options: Dict[str, Any]) -> None: + exclusive_primary_alt_value_setting( + warn_error_options, "include", "error", "warn_error_options" + ) + exclusive_primary_alt_value_setting( + warn_error_options, "exclude", "warn", "warn_error_options" + ) + for key in ("include", "exclude", "silence"): + if key in warn_error_options and warn_error_options[key] is None: + warn_error_options[key] = [] diff --git a/tests/functional/configs/test_warn_error_options.py b/tests/functional/configs/test_warn_error_options.py index fd4438ecfae..0c69d53a48e 100644 --- a/tests/functional/configs/test_warn_error_options.py +++ b/tests/functional/configs/test_warn_error_options.py @@ -2,7 +2,8 @@ from dbt.cli.main import dbtRunner, dbtRunnerResult from dbt.events.types import DeprecatedModel -from dbt.tests.util import update_config_file +from dbt.flags import get_flags +from dbt.tests.util import run_dbt, update_config_file from dbt_common.events.base_types import EventLevel from tests.functional.utils import EventCatcher from typing import Dict, Union @@ -201,3 +202,29 @@ def test_cant_set_both_exclude_and_warn( assert not result.success assert result.exception is not None assert "Only `warn` or `exclude` can be specified" in str(result.exception) + + +class TestEmptyWarnError: + @pytest.fixture(scope="class") + def models(self): + return {"my_model.sql": my_model_sql, "schema.yml": schema_yml} + + # This tests for a bug in creating WarnErrorOptions when warn or + # error are set to None (in yaml = warn:) + def test_project_flags(self, project): + project_flags = { + "flags": { + "send_anonymous_usage_stats": False, + "warn_error_options": { + "warn": None, + "error": None, + "silence": ["TestsConfigDeprecation"], + }, + } + } + update_config_file(project_flags, project.project_root, "dbt_project.yml") + run_dbt(["run"]) + flags = get_flags() + # Note: WarnErrorOptions is not a dataclass, so you won't get "silence" + # from to_dict or stringifying. + assert flags.warn_error_options.silence == ["TestsConfigDeprecation"] diff --git a/tests/unit/config/test_utils.py b/tests/unit/config/test_utils.py index d2d7f99499d..88f73ad2fdb 100644 --- a/tests/unit/config/test_utils.py +++ b/tests/unit/config/test_utils.py @@ -1,6 +1,9 @@ import pytest -from dbt.config.utils import exclusive_primary_alt_value_setting +from dbt.config.utils import ( + exclusive_primary_alt_value_setting, + normalize_warn_error_options, +) from dbt.exceptions import DbtExclusivePropertyUseError @@ -27,7 +30,6 @@ def test_alt_set(self, primary_key: str, alt_key: str, value: str): test_dict = {alt_key: value} exclusive_primary_alt_value_setting(test_dict, primary_key, alt_key) assert test_dict.get(primary_key) == value - assert test_dict.get(alt_key) == value def test_primary_and_alt_set(self, primary_key: str, alt_key: str, value: str): test_dict = {primary_key: value, alt_key: value} @@ -39,3 +41,34 @@ def test_neither_primary_nor_alt_set(self, primary_key: str, alt_key: str): exclusive_primary_alt_value_setting(test_dict, primary_key, alt_key) assert test_dict.get(primary_key) is None assert test_dict.get(alt_key) is None + + +class TestNormalizeWarnErrorOptions: + def test_primary_set(self): + test_dict = { + "error": ["SomeWarning"], + } + normalize_warn_error_options(test_dict) + assert len(test_dict) == 1 + assert test_dict["include"] == ["SomeWarning"] + + def test_convert(self): + test_dict = {"warn": None, "silence": None, "include": ["SomeWarning"]} + normalize_warn_error_options(test_dict) + assert test_dict["exclude"] == [] + assert test_dict["include"] == ["SomeWarning"] + assert test_dict["silence"] == [] + + def test_both_keys_set(self): + test_dict = { + "warn": ["SomeWarning"], + "exclude": ["SomeWarning"], + } + with pytest.raises(DbtExclusivePropertyUseError): + normalize_warn_error_options(test_dict) + + def test_empty_dict(self): + test_dict = {} + normalize_warn_error_options(test_dict) + assert test_dict.get("include") is None + assert test_dict.get("exclude") is None