Skip to content

Commit

Permalink
Fix warn error options being invalid when warn or error set to none (#…
Browse files Browse the repository at this point in the history
…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 <[email protected]>
  • Loading branch information
QMalcolm and gshank authored Jul 18, 2024
1 parent 23545fe commit 2a8b0d6
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 23 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20240716-171427.yaml
Original file line number Diff line number Diff line change
@@ -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"
9 changes: 2 additions & 7 deletions core/dbt/cli/option_types.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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", []),
Expand Down
17 changes: 6 additions & 11 deletions core/dbt/config/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,17 @@

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,
PACKAGE_LOCK_HASH_KEY,
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,
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
17 changes: 15 additions & 2 deletions core/dbt/config/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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] = []
29 changes: 28 additions & 1 deletion tests/functional/configs/test_warn_error_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"]
37 changes: 35 additions & 2 deletions tests/unit/config/test_utils.py
Original file line number Diff line number Diff line change
@@ -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


Expand All @@ -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}
Expand All @@ -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

0 comments on commit 2a8b0d6

Please sign in to comment.