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

Add ability to silence warnings AND allow error and warn to be used for include and exclude #10058

Merged
merged 17 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8e5432d
Refactor test class `EventCatcher` into utils to make accessible to o…
QMalcolm Apr 24, 2024
3292c3a
Raise minimum version of dbt_common to 1.0.2
QMalcolm Apr 25, 2024
f892f3d
Add ability to silence warnings from `warn_error_options` CLI arguments
QMalcolm Apr 25, 2024
4a94d6a
Add `flush` to `EventCatcher` test util, and use in `test_warn_error_…
QMalcolm Apr 25, 2024
e02df2d
Add tests to `TestWarnErrorOptionsFromCLI` for `include` and `exclude`
QMalcolm Apr 25, 2024
9f05e1f
Refactor `runner` and `catcher` into fixtures for `TestWarnErrorOptio…
QMalcolm Apr 25, 2024
c52d653
Test support for setting `silence` of `warn_error_options` in `dbt_pr…
QMalcolm Apr 25, 2024
cf72bf1
Add tests to `TestWarnErrorOptionsFromProject` for `include` and `exc…
QMalcolm Apr 25, 2024
4c7e04b
Abstract repetitive assertion logic in `test_warn_error_options.py`
QMalcolm Apr 25, 2024
ebf9106
Update `warn_error_options` in CLI args to support `error` and `warn`…
QMalcolm Apr 26, 2024
3eb4e0c
Update `warn_error_options` in Project flags to support `error` and `…
QMalcolm Apr 26, 2024
53d61f9
Use `DbtWarnErrorOptionsError` in `exclusive_primary_alt_value_settin…
QMalcolm Apr 26, 2024
0e87a17
Add changie documentation for 9644 features
QMalcolm Apr 27, 2024
49b66e1
Add unit tests for `exclusive_primary_alt_value_setting` method
QMalcolm Apr 29, 2024
e1c7839
Fixup typo in changie doc `Features-20240426-233126.yaml`
QMalcolm Apr 30, 2024
0d960b2
Update `exclusive_primary_alt_value_setting` to handle `None` dictionary
QMalcolm Apr 30, 2024
41e6d7c
Update `exclusive_primary_alt_value_setting` to raise more generalize…
QMalcolm Apr 30, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we bumping a patch pin?

Copy link
Contributor Author

@QMalcolm QMalcolm Apr 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The addition of the attr silence to the class WarnErrorOptions is new, and got released in dbt-common 1.0.2. If we don't bump the minimum, then it would be possible for us to get dbt-common 1.0.1 which doesn't support silence, and we'd break at run time. Here is the work in dbt-common which added the functionality

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started a thread in slack about this. Unrelated to you actual PR - more around how this should have been a minor bump instead of a patch.

"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