Skip to content

Commit

Permalink
Add ability to silence warnings AND allow error and warn to be …
Browse files Browse the repository at this point in the history
…used for `include` and `exclude` (#10058)

* Refactor test class `EventCatcher` into utils to make accessible to other tests

* Raise minimum version of dbt_common to 1.0.2

We're going to start depending on `silence` existing as a attr of
`WarnErrorOptions`. The `silence` attr was only added in dbt_common
1.0.2, thus that is our new minimum.

* Add ability to silence warnings from `warn_error_options` CLI arguments

* Add `flush` to `EventCatcher` test util, and use in `test_warn_error_options`

* Add tests to `TestWarnErrorOptionsFromCLI` for `include` and `exclude`

* Test support for setting `silence` of `warn_error_options` in `dbt_project` flags

Support for `silence` was _automatically_ added when we upgraded to dbt_common 1.0.2.
This is because we build the project flags in a `.from_dict` style, which is cool. In
this case it was _automatically_ handled in `read_project_flags` in `project.py`. More
specifically here https://github.com/dbt-labs/dbt-core/blob/bcbde3ac4204f00d964a3ea60896b6af1df18c71/core/dbt/config/project.py#L839

* Add tests to `TestWarnErrorOptionsFromProject` for `include` and `exclude`

Typically we can't have multiple tests in the same `test class` if they're
utilizing/modifying file system fixtures. That is because the file system
fixtures are scoped to test classes, so they don't reset inbetween tests within
the same test class. This problem _was_ affectin these tests as they modify the
`dbt_project.yml` file which is set by a class based fixture. To get around this,
because I find it annoying to create multiple test classes when the tests really
should be grouped, I created a "function" scoped fixture to reset the `dbt_project.yml`.

* 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.

* Update `warn_error_options` in Project flags to support `error` and `warn` options

As part of this I refactored `exclusive_primary_alt_value_setting` into an upstream
location `/config/utils`. That is because importing it in `/config/project.py` from
`cli/option_types.py` caused some circular dependency issues.

* Use `DbtExclusivePropertyUseError` in `exclusive_primary_alt_value_setting` instead of `DbtConfigError`

Using `DbtConfigError` seemed reasonable. However in order to make sure the error
got raised in `read_project_flags` we had to mark it to be `DbtConfigError`s to be
re-raised. This had the uninteded consequence of reraising a smattering of errors
which were previously being swallowed. I'd argue that if those are errors we're
swallowing, the functions that raise those errors should be modified perhaps to
conditionally not raise them, but that's not the world we live in and is out of
scope for this branch of work. Thus instead we've created a error specific to
`WarnErrorOptions` issues, and now use, and catch for re-raising.


* Add unit tests for `exclusive_primary_alt_value_setting` method

I debated about parametrizing these tests, and it can be done. However,
I found that the resulting code ended up being about the same number of
lines and slightly less readable (in my opinion). Given the simplicity of
these tests, I think not parametrizing them is okay.
  • Loading branch information
QMalcolm authored Apr 30, 2024
1 parent 35062ca commit 1a9fb61
Show file tree
Hide file tree
Showing 11 changed files with 336 additions and 21 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Features-20240426-233126.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: Ability to `silence` warnings via `warn_error_options`
time: 2024-04-26T23:31:26.601057-05:00
custom:
Author: QMalcolm
Issue: "9644"
6 changes: 6 additions & 0 deletions .changes/unreleased/Features-20240426-233208.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: Allow aliases `error` for `include` and `warn` for `exclude` in `warn_error_options`
time: 2024-04-26T23:32:08.771114-05:00
custom:
Author: QMalcolm
Issue: "9644"
10 changes: 8 additions & 2 deletions core/dbt/cli/option_types.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
from click import ParamType, Choice

from dbt.config.utils import parse_cli_yaml_string
from dbt.config.utils import parse_cli_yaml_string, exclusive_primary_alt_value_setting
from dbt.events import ALL_EVENT_NAMES
from dbt.exceptions import ValidationError, OptionNotYamlDictError
from dbt_common.exceptions import DbtValidationError

from dbt_common.helper_types import WarnErrorOptions


Expand Down Expand Up @@ -52,10 +51,17 @@ 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"
)

return WarnErrorOptions(
include=include_exclude.get("include", []),
exclude=include_exclude.get("exclude", []),
silence=include_exclude.get("silence", []),
valid_error_names=ALL_EVENT_NAMES,
)

Expand Down
17 changes: 15 additions & 2 deletions core/dbt/config/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from dbt.adapters.contracts.connection import QueryComment
from dbt.exceptions import (
DbtProjectError,
DbtExclusivePropertyUseError,
ProjectContractBrokenError,
ProjectContractError,
DbtRuntimeError,
Expand All @@ -39,6 +40,7 @@
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 @@ -835,10 +837,21 @@ def read_project_flags(project_dir: str, profiles_dir: str) -> ProjectFlags:
project_flags = profile_project_flags

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"
)

ProjectFlags.validate(project_flags)
return ProjectFlags.from_dict(project_flags)
except (DbtProjectError) as exc:
# We don't want to eat the DbtProjectError for UserConfig to ProjectFlags
except (DbtProjectError, DbtExclusivePropertyUseError) as exc:
# We don't want to eat the DbtProjectError for UserConfig to ProjectFlags or
# DbtConfigError for warn_error_options munging
raise exc
except (DbtRuntimeError, ValidationError):
pass
Expand Down
33 changes: 31 additions & 2 deletions core/dbt/config/utils.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
from typing import Any, Dict
from typing import Any, Dict, Optional


from dbt.clients import yaml_helper
from dbt_common.events.functions import fire_event
from dbt.events.types import InvalidOptionYAML
from dbt.exceptions import OptionNotYamlDictError
from dbt.exceptions import DbtExclusivePropertyUseError, OptionNotYamlDictError
from dbt_common.exceptions import DbtValidationError


Expand All @@ -23,3 +23,32 @@ def parse_cli_yaml_string(var_string: str, cli_option_name: str) -> Dict[str, An
except (DbtValidationError, OptionNotYamlDictError):
fire_event(InvalidOptionYAML(option_name=cli_option_name))
raise


def exclusive_primary_alt_value_setting(
dictionary: Optional[Dict[str, Any]],
primary: str,
alt: str,
parent_config: Optional[str] = None,
) -> 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.
"""

if dictionary is None:
return

primary_options = dictionary.get(primary)
alt_options = dictionary.get(alt)

if primary_options and alt_options:
where = f" in `{parent_config}`" if parent_config is not None else ""
raise DbtExclusivePropertyUseError(
f"Only `{alt}` or `{primary}` can be specified{where}, not both"
)

if alt_options:
dictionary[primary] = alt_options
4 changes: 4 additions & 0 deletions core/dbt/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ class DbtProfileError(DbtConfigError):
pass


class DbtExclusivePropertyUseError(DbtConfigError):
pass


class InvalidSelectorError(DbtRuntimeError):
def __init__(self, name: str) -> None:
self.name = name
Expand Down
2 changes: 1 addition & 1 deletion core/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
"minimal-snowplow-tracker>=0.0.2,<0.1",
"dbt-semantic-interfaces>=0.5.1,<0.6",
# Minor versions for these are expected to be backwards-compatible
"dbt-common>=1.0.1,<2.0",
"dbt-common>=1.0.2,<2.0",
"dbt-adapters>=0.1.0a2,<2.0",
# ----
# Expect compatibility with all new versions of these packages, so lower bounds only.
Expand Down
205 changes: 205 additions & 0 deletions tests/functional/configs/test_warn_error_options.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
import pytest

from dbt.cli.main import dbtRunner, dbtRunnerResult
from dbt.events.types import DeprecatedModel
from dbt.tests.util import update_config_file
from dbt_common.events.base_types import EventLevel
from tests.functional.utils import EventCatcher
from typing import Dict, Union

ModelsDictSpec = Dict[str, Union[str, "ModelsDictSpec"]]

my_model_sql = """SELECT 1 AS id, 'cats are cute' AS description"""
schema_yml = """
version: 2
models:
- name: my_model
deprecation_date: 2020-01-01
"""


@pytest.fixture(scope="class")
def models() -> ModelsDictSpec:
return {"my_model.sql": my_model_sql, "schema.yml": schema_yml}


@pytest.fixture(scope="function")
def catcher() -> EventCatcher:
return EventCatcher(event_to_catch=DeprecatedModel)


@pytest.fixture(scope="function")
def runner(catcher: EventCatcher) -> dbtRunner:
return dbtRunner(callbacks=[catcher.catch])


def assert_deprecation_warning(result: dbtRunnerResult, catcher: EventCatcher) -> None:
assert result.success
assert result.exception is None
assert len(catcher.caught_events) == 1
assert catcher.caught_events[0].info.level == EventLevel.WARN.value


def assert_deprecation_error(result: dbtRunnerResult) -> None:
assert not result.success
assert result.exception is not None
assert "Model my_model has passed its deprecation date of" in str(result.exception)


class TestWarnErrorOptionsFromCLI:
def test_can_silence(self, project, catcher: EventCatcher, runner: dbtRunner) -> None:
result = runner.invoke(["run"])
assert_deprecation_warning(result, catcher)

catcher.flush()
runner.invoke(
["run", "--warn-error-options", "{'include': 'all', 'silence': ['DeprecatedModel']}"]
)
assert len(catcher.caught_events) == 0

def test_can_raise_warning_to_error(
self, project, catcher: EventCatcher, runner: dbtRunner
) -> None:
result = runner.invoke(["run"])
assert_deprecation_warning(result, catcher)

catcher.flush()
result = runner.invoke(["run", "--warn-error-options", "{'include': ['DeprecatedModel']}"])
assert_deprecation_error(result)

catcher.flush()
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:
result = runner.invoke(["run", "--warn-error-options", "{'include': 'all'}"])
assert_deprecation_error(result)

catcher.flush()
result = runner.invoke(
["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")
def clear_project_flags(self, project_root) -> None:
flags = {"flags": {}}
update_config_file(flags, project_root, "dbt_project.yml")

def test_can_silence(
self, project, clear_project_flags, project_root, catcher: EventCatcher, runner: dbtRunner
) -> None:
result = runner.invoke(["run"])
assert_deprecation_warning(result, catcher)

silence_options = {
"flags": {"warn_error_options": {"include": "all", "silence": ["DeprecatedModel"]}}
}
update_config_file(silence_options, project_root, "dbt_project.yml")

catcher.flush()
runner.invoke(["run"])
assert len(catcher.caught_events) == 0

def test_can_raise_warning_to_error(
self, project, clear_project_flags, project_root, catcher: EventCatcher, runner: dbtRunner
) -> None:
result = runner.invoke(["run"])
assert_deprecation_warning(result, catcher)

include_options = {"flags": {"warn_error_options": {"include": ["DeprecatedModel"]}}}
update_config_file(include_options, project_root, "dbt_project.yml")

catcher.flush()
result = runner.invoke(["run"])
assert_deprecation_error(result)

include_options = {"flags": {"warn_error_options": {"include": "all"}}}
update_config_file(include_options, project_root, "dbt_project.yml")

catcher.flush()
result = runner.invoke(["run"])
assert_deprecation_error(result)

def test_can_exclude_specific_event(
self, project, clear_project_flags, project_root, catcher: EventCatcher, runner: dbtRunner
) -> None:
include_options = {"flags": {"warn_error_options": {"include": "all"}}}
update_config_file(include_options, project_root, "dbt_project.yml")
result = runner.invoke(["run"])
assert_deprecation_error(result)

exclude_options = {
"flags": {"warn_error_options": {"include": "all", "exclude": ["DeprecatedModel"]}}
}
update_config_file(exclude_options, project_root, "dbt_project.yml")

catcher.flush()
result = runner.invoke(["run"])
assert_deprecation_warning(result, catcher)

def test_cant_set_both_include_and_error(
self, project, clear_project_flags, project_root, runner: dbtRunner
) -> None:
exclude_options = {"flags": {"warn_error_options": {"include": "all", "error": "all"}}}
update_config_file(exclude_options, project_root, "dbt_project.yml")
result = runner.invoke(["run"])
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, clear_project_flags, project_root, runner: dbtRunner
) -> None:
exclude_options = {
"flags": {
"warn_error_options": {
"include": "all",
"exclude": ["DeprecatedModel"],
"warn": ["DeprecatedModel"],
}
}
}
update_config_file(exclude_options, project_root, "dbt_project.yml")
result = runner.invoke(["run"])
assert not result.success
assert result.exception is not None
assert "Only `warn` or `exclude` can be specified" in str(result.exception)
Original file line number Diff line number Diff line change
@@ -1,21 +1,11 @@
import pytest

from dataclasses import dataclass, field
from dbt.cli.main import dbtRunner
from dbt_common.events.base_types import BaseEvent, EventLevel, EventMsg
from dbt_common.events.base_types import EventLevel
from dbt.events.types import SpacesInModelNameDeprecation, TotalModelNamesWithSpacesDeprecation
from dbt.tests.util import update_config_file
from typing import Dict, List


@dataclass
class EventCatcher:
event_to_catch: BaseEvent
caught_events: List[EventMsg] = field(default_factory=list)

def catch(self, event: EventMsg):
if event.info.name == self.event_to_catch.__name__:
self.caught_events.append(event)
from tests.functional.utils import EventCatcher
from typing import Dict


class TestSpacesInModelNamesHappyPath:
Expand Down
Loading

0 comments on commit 1a9fb61

Please sign in to comment.