From 8e5432d2a730d140e8b67429a31f240de89e1789 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Wed, 24 Apr 2024 16:45:01 -0700 Subject: [PATCH 01/17] Refactor test class `EventCatcher` into utils to make accessible to other tests --- .../test_check_for_spaces_in_model_names.py | 16 +++------------- tests/functional/utils.py | 14 +++++++++++++- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/tests/functional/manifest_validations/test_check_for_spaces_in_model_names.py b/tests/functional/manifest_validations/test_check_for_spaces_in_model_names.py index 45ca4bab307..4fe10f43f16 100644 --- a/tests/functional/manifest_validations/test_check_for_spaces_in_model_names.py +++ b/tests/functional/manifest_validations/test_check_for_spaces_in_model_names.py @@ -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: diff --git a/tests/functional/utils.py b/tests/functional/utils.py index a82aa378e43..dbbb25da193 100644 --- a/tests/functional/utils.py +++ b/tests/functional/utils.py @@ -1,7 +1,9 @@ import os from contextlib import contextmanager +from dataclasses import dataclass, field from datetime import datetime -from typing import Optional +from dbt_common.events.base_types import BaseEvent, EventMsg +from typing import List, Optional from pathlib import Path @@ -17,3 +19,13 @@ def up_one(return_path: Optional[Path] = None): def is_aware(dt: datetime) -> bool: return dt.tzinfo is not None and dt.tzinfo.utcoffset(dt) is not None + + +@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 3292c3afee86159e7c96de0a914b38011d628d12 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Thu, 25 Apr 2024 16:02:01 -0700 Subject: [PATCH 02/17] 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. --- core/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/setup.py b/core/setup.py index 1f103c19534..88c380c8ef6 100644 --- a/core/setup.py +++ b/core/setup.py @@ -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. From f892f3d12631534ed91b50dfaaff8f62543d215b Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Thu, 25 Apr 2024 09:03:11 -0700 Subject: [PATCH 03/17] Add ability to silence warnings from `warn_error_options` CLI arguments --- core/dbt/cli/option_types.py | 1 + .../configs/test_warn_error_options.py | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 tests/functional/configs/test_warn_error_options.py diff --git a/core/dbt/cli/option_types.py b/core/dbt/cli/option_types.py index d55aa736e16..71291177c0c 100644 --- a/core/dbt/cli/option_types.py +++ b/core/dbt/cli/option_types.py @@ -56,6 +56,7 @@ def convert(self, value, param, ctx): return WarnErrorOptions( include=include_exclude.get("include", []), exclude=include_exclude.get("exclude", []), + silence=include_exclude.get("silence", []), valid_error_names=ALL_EVENT_NAMES, ) diff --git a/tests/functional/configs/test_warn_error_options.py b/tests/functional/configs/test_warn_error_options.py new file mode 100644 index 00000000000..0881ca46dfd --- /dev/null +++ b/tests/functional/configs/test_warn_error_options.py @@ -0,0 +1,34 @@ +import pytest + +from dbt.cli.main import dbtRunner +from dbt.events.types import DeprecatedModel +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 +""" + + +class TestWarnErrorOptionsFromCLI: + @pytest.fixture(scope="class") + def models(self) -> ModelsDictSpec: + return {"my_model.sql": my_model_sql, "schema.yml": schema_yml} + + def test_can_silence(self, project) -> None: + catcher = EventCatcher(event_to_catch=DeprecatedModel) + runner = dbtRunner(callbacks=[catcher.catch]) + runner.invoke(["run"]) + assert len(catcher.caught_events) == 1 + + catcher.caught_events = [] + runner.invoke( + ["run", "--warn-error-options", "{'include': 'all', 'silence': ['DeprecatedModel']}"] + ) + assert len(catcher.caught_events) == 0 From 4a94d6a3abe7ffc586dbe06ec644ecd9d4a2340d Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Thu, 25 Apr 2024 09:06:13 -0700 Subject: [PATCH 04/17] Add `flush` to `EventCatcher` test util, and use in `test_warn_error_options` --- tests/functional/configs/test_warn_error_options.py | 2 +- tests/functional/utils.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/functional/configs/test_warn_error_options.py b/tests/functional/configs/test_warn_error_options.py index 0881ca46dfd..f8aec2eb6e6 100644 --- a/tests/functional/configs/test_warn_error_options.py +++ b/tests/functional/configs/test_warn_error_options.py @@ -27,7 +27,7 @@ def test_can_silence(self, project) -> None: runner.invoke(["run"]) assert len(catcher.caught_events) == 1 - catcher.caught_events = [] + catcher.flush() runner.invoke( ["run", "--warn-error-options", "{'include': 'all', 'silence': ['DeprecatedModel']}"] ) diff --git a/tests/functional/utils.py b/tests/functional/utils.py index dbbb25da193..a5c705780b7 100644 --- a/tests/functional/utils.py +++ b/tests/functional/utils.py @@ -29,3 +29,6 @@ class EventCatcher: def catch(self, event: EventMsg): if event.info.name == self.event_to_catch.__name__: self.caught_events.append(event) + + def flush(self) -> None: + self.caught_events = [] From e02df2d5c188dafd4c6217c36fa652b089e20468 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Thu, 25 Apr 2024 09:26:44 -0700 Subject: [PATCH 05/17] Add tests to `TestWarnErrorOptionsFromCLI` for `include` and `exclude` --- .../configs/test_warn_error_options.py | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/functional/configs/test_warn_error_options.py b/tests/functional/configs/test_warn_error_options.py index f8aec2eb6e6..2562f231cd4 100644 --- a/tests/functional/configs/test_warn_error_options.py +++ b/tests/functional/configs/test_warn_error_options.py @@ -2,6 +2,7 @@ from dbt.cli.main import dbtRunner from dbt.events.types import DeprecatedModel +from dbt_common.events.base_types import EventLevel from tests.functional.utils import EventCatcher from typing import Dict, Union @@ -32,3 +33,42 @@ def test_can_silence(self, project) -> None: ["run", "--warn-error-options", "{'include': 'all', 'silence': ['DeprecatedModel']}"] ) assert len(catcher.caught_events) == 0 + + def test_can_raise_warning_to_error(self, project) -> None: + catcher = EventCatcher(event_to_catch=DeprecatedModel) + runner = dbtRunner(callbacks=[catcher.catch]) + + result = runner.invoke(["run"]) + assert result.success + assert result.exception is None + assert len(catcher.caught_events) == 1 + assert catcher.caught_events[0].info.level == EventLevel.WARN.value + + catcher.flush() + result = runner.invoke(["run", "--warn-error-options", "{'include': ['DeprecatedModel']}"]) + assert not result.success + assert result.exception is not None + assert "Model my_model has passed its deprecation date of" in str(result.exception) + + catcher.flush() + result = runner.invoke(["run", "--warn-error-options", "{'include': 'all'}"]) + assert not result.success + assert result.exception is not None + assert "Model my_model has passed its deprecation date of" in str(result.exception) + + def test_can_exclude_specific_event(self, project) -> None: + catcher = EventCatcher(event_to_catch=DeprecatedModel) + runner = dbtRunner(callbacks=[catcher.catch]) + result = runner.invoke(["run", "--warn-error-options", "{'include': 'all'}"]) + assert not result.success + assert result.exception is not None + assert "Model my_model has passed its deprecation date of" in str(result.exception) + + catcher.flush() + result = runner.invoke( + ["run", "--warn-error-options", "{'include': 'all', exclude: ['DeprecatedModel']}"] + ) + assert result.success + assert result.exception is None + assert len(catcher.caught_events) == 1 + assert catcher.caught_events[0].info.level == EventLevel.WARN.value From 9f05e1f76b929c467165da78d22f58f6c2d88a6b Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Thu, 25 Apr 2024 09:32:45 -0700 Subject: [PATCH 06/17] Refactor `runner` and `catcher` into fixtures for `TestWarnErrorOptionsFromCLI` --- .../configs/test_warn_error_options.py | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/tests/functional/configs/test_warn_error_options.py b/tests/functional/configs/test_warn_error_options.py index 2562f231cd4..1a862470bb8 100644 --- a/tests/functional/configs/test_warn_error_options.py +++ b/tests/functional/configs/test_warn_error_options.py @@ -22,9 +22,15 @@ class TestWarnErrorOptionsFromCLI: def models(self) -> ModelsDictSpec: return {"my_model.sql": my_model_sql, "schema.yml": schema_yml} - def test_can_silence(self, project) -> None: - catcher = EventCatcher(event_to_catch=DeprecatedModel) - runner = dbtRunner(callbacks=[catcher.catch]) + @pytest.fixture(scope="function") + def catcher(self) -> EventCatcher: + return EventCatcher(event_to_catch=DeprecatedModel) + + @pytest.fixture(scope="function") + def runner(self, catcher: EventCatcher) -> dbtRunner: + return dbtRunner(callbacks=[catcher.catch]) + + def test_can_silence(self, project, catcher: EventCatcher, runner: dbtRunner) -> None: runner.invoke(["run"]) assert len(catcher.caught_events) == 1 @@ -34,10 +40,9 @@ def test_can_silence(self, project) -> None: ) assert len(catcher.caught_events) == 0 - def test_can_raise_warning_to_error(self, project) -> None: - catcher = EventCatcher(event_to_catch=DeprecatedModel) - runner = dbtRunner(callbacks=[catcher.catch]) - + def test_can_raise_warning_to_error( + self, project, catcher: EventCatcher, runner: dbtRunner + ) -> None: result = runner.invoke(["run"]) assert result.success assert result.exception is None @@ -56,9 +61,9 @@ def test_can_raise_warning_to_error(self, project) -> None: assert result.exception is not None assert "Model my_model has passed its deprecation date of" in str(result.exception) - def test_can_exclude_specific_event(self, project) -> None: - catcher = EventCatcher(event_to_catch=DeprecatedModel) - runner = dbtRunner(callbacks=[catcher.catch]) + def test_can_exclude_specific_event( + self, project, catcher: EventCatcher, runner: dbtRunner + ) -> None: result = runner.invoke(["run", "--warn-error-options", "{'include': 'all'}"]) assert not result.success assert result.exception is not None From c52d6531fcc0ca51804906dccb726d68be8550ea Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Thu, 25 Apr 2024 15:40:06 -0700 Subject: [PATCH 07/17] 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 --- .../configs/test_warn_error_options.py | 44 ++++++++++++++----- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/tests/functional/configs/test_warn_error_options.py b/tests/functional/configs/test_warn_error_options.py index 1a862470bb8..8e97a1b4fcb 100644 --- a/tests/functional/configs/test_warn_error_options.py +++ b/tests/functional/configs/test_warn_error_options.py @@ -2,6 +2,7 @@ from dbt.cli.main import dbtRunner 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 @@ -17,19 +18,22 @@ """ -class TestWarnErrorOptionsFromCLI: - @pytest.fixture(scope="class") - def models(self) -> ModelsDictSpec: - return {"my_model.sql": my_model_sql, "schema.yml": schema_yml} +@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 catcher(self) -> EventCatcher: - return EventCatcher(event_to_catch=DeprecatedModel) - @pytest.fixture(scope="function") - def runner(self, catcher: EventCatcher) -> dbtRunner: - return dbtRunner(callbacks=[catcher.catch]) +@pytest.fixture(scope="function") +def runner(catcher: EventCatcher) -> dbtRunner: + return dbtRunner(callbacks=[catcher.catch]) + +class TestWarnErrorOptionsFromCLI: def test_can_silence(self, project, catcher: EventCatcher, runner: dbtRunner) -> None: runner.invoke(["run"]) assert len(catcher.caught_events) == 1 @@ -77,3 +81,23 @@ def test_can_exclude_specific_event( assert result.exception is None assert len(catcher.caught_events) == 1 assert catcher.caught_events[0].info.level == EventLevel.WARN.value + + +class TestWarnErrorOptionsFromProject: + def test_can_silence( + self, project, project_root, catcher: EventCatcher, runner: dbtRunner + ) -> None: + result = runner.invoke(["run"]) + assert result.success + assert result.exception is None + assert len(catcher.caught_events) == 1 + assert catcher.caught_events[0].info.level == EventLevel.WARN.value + + 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 From cf72bf17cc9cebbda191d4d3d4704d47546720a8 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Thu, 25 Apr 2024 16:18:13 -0700 Subject: [PATCH 08/17] 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`. --- .../configs/test_warn_error_options.py | 56 ++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/tests/functional/configs/test_warn_error_options.py b/tests/functional/configs/test_warn_error_options.py index 8e97a1b4fcb..1384217f997 100644 --- a/tests/functional/configs/test_warn_error_options.py +++ b/tests/functional/configs/test_warn_error_options.py @@ -84,8 +84,13 @@ def test_can_exclude_specific_event( 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, project_root, catcher: EventCatcher, runner: dbtRunner + self, project, clear_project_flags, project_root, catcher: EventCatcher, runner: dbtRunner ) -> None: result = runner.invoke(["run"]) assert result.success @@ -101,3 +106,52 @@ def test_can_silence( 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 result.success + assert result.exception is None + assert len(catcher.caught_events) == 1 + assert catcher.caught_events[0].info.level == EventLevel.WARN.value + + 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 not result.success + assert result.exception is not None + assert "Model my_model has passed its deprecation date of" in str(result.exception) + + 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 not result.success + assert result.exception is not None + assert "Model my_model has passed its deprecation date of" in str(result.exception) + + 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 not result.success + assert result.exception is not None + assert "Model my_model has passed its deprecation date of" in str(result.exception) + + 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 result.success + assert result.exception is None + assert len(catcher.caught_events) == 1 + assert catcher.caught_events[0].info.level == EventLevel.WARN.value From 4c7e04bf50a722b735d44cafb9a6045a90a672c6 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Thu, 25 Apr 2024 16:28:35 -0700 Subject: [PATCH 09/17] Abstract repetitive assertion logic in `test_warn_error_options.py` --- .../configs/test_warn_error_options.py | 68 ++++++++----------- 1 file changed, 27 insertions(+), 41 deletions(-) diff --git a/tests/functional/configs/test_warn_error_options.py b/tests/functional/configs/test_warn_error_options.py index 1384217f997..24051979de4 100644 --- a/tests/functional/configs/test_warn_error_options.py +++ b/tests/functional/configs/test_warn_error_options.py @@ -1,6 +1,6 @@ import pytest -from dbt.cli.main import dbtRunner +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 @@ -33,10 +33,23 @@ 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: - runner.invoke(["run"]) - assert len(catcher.caught_events) == 1 + result = runner.invoke(["run"]) + assert_deprecation_warning(result, catcher) catcher.flush() runner.invoke( @@ -48,39 +61,27 @@ def test_can_raise_warning_to_error( self, project, catcher: EventCatcher, runner: dbtRunner ) -> None: result = runner.invoke(["run"]) - assert result.success - assert result.exception is None - assert len(catcher.caught_events) == 1 - assert catcher.caught_events[0].info.level == EventLevel.WARN.value + assert_deprecation_warning(result, catcher) catcher.flush() result = runner.invoke(["run", "--warn-error-options", "{'include': ['DeprecatedModel']}"]) - assert not result.success - assert result.exception is not None - assert "Model my_model has passed its deprecation date of" in str(result.exception) + assert_deprecation_error(result) catcher.flush() result = runner.invoke(["run", "--warn-error-options", "{'include': 'all'}"]) - assert not result.success - assert result.exception is not None - assert "Model my_model has passed its deprecation date of" in str(result.exception) + 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 not result.success - assert result.exception is not None - assert "Model my_model has passed its deprecation date of" in str(result.exception) + assert_deprecation_error(result) catcher.flush() result = runner.invoke( ["run", "--warn-error-options", "{'include': 'all', exclude: ['DeprecatedModel']}"] ) - assert result.success - assert result.exception is None - assert len(catcher.caught_events) == 1 - assert catcher.caught_events[0].info.level == EventLevel.WARN.value + assert_deprecation_warning(result, catcher) class TestWarnErrorOptionsFromProject: @@ -93,10 +94,7 @@ def test_can_silence( self, project, clear_project_flags, project_root, catcher: EventCatcher, runner: dbtRunner ) -> None: result = runner.invoke(["run"]) - assert result.success - assert result.exception is None - assert len(catcher.caught_events) == 1 - assert catcher.caught_events[0].info.level == EventLevel.WARN.value + assert_deprecation_warning(result, catcher) silence_options = { "flags": {"warn_error_options": {"include": "all", "silence": ["DeprecatedModel"]}} @@ -111,28 +109,21 @@ def test_can_raise_warning_to_error( self, project, clear_project_flags, project_root, catcher: EventCatcher, runner: dbtRunner ) -> None: result = runner.invoke(["run"]) - assert result.success - assert result.exception is None - assert len(catcher.caught_events) == 1 - assert catcher.caught_events[0].info.level == EventLevel.WARN.value + 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 not result.success - assert result.exception is not None - assert "Model my_model has passed its deprecation date of" in str(result.exception) + 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 not result.success - assert result.exception is not None - assert "Model my_model has passed its deprecation date of" in str(result.exception) + assert_deprecation_error(result) def test_can_exclude_specific_event( self, project, clear_project_flags, project_root, catcher: EventCatcher, runner: dbtRunner @@ -140,9 +131,7 @@ def test_can_exclude_specific_event( include_options = {"flags": {"warn_error_options": {"include": "all"}}} update_config_file(include_options, project_root, "dbt_project.yml") result = runner.invoke(["run"]) - assert not result.success - assert result.exception is not None - assert "Model my_model has passed its deprecation date of" in str(result.exception) + assert_deprecation_error(result) exclude_options = { "flags": {"warn_error_options": {"include": "all", "exclude": ["DeprecatedModel"]}} @@ -151,7 +140,4 @@ def test_can_exclude_specific_event( catcher.flush() result = runner.invoke(["run"]) - assert result.success - assert result.exception is None - assert len(catcher.caught_events) == 1 - assert catcher.caught_events[0].info.level == EventLevel.WARN.value + assert_deprecation_warning(result, catcher) From ebf910640f5bd142884f157a93c36645233d4573 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Thu, 25 Apr 2024 17:37:26 -0700 Subject: [PATCH 10/17] 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") From 3eb4e0ca58d49f117ad943380ec2d4cfec19171b Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Fri, 26 Apr 2024 09:27:17 -0700 Subject: [PATCH 11/17] 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. --- core/dbt/cli/option_types.py | 27 ++---------------- core/dbt/config/project.py | 14 ++++++++-- core/dbt/config/utils.py | 24 +++++++++++++++- .../configs/test_warn_error_options.py | 28 +++++++++++++++++++ 4 files changed, 65 insertions(+), 28 deletions(-) diff --git a/core/dbt/cli/option_types.py b/core/dbt/cli/option_types.py index b95fb3b9d64..ef04e348260 100644 --- a/core/dbt/cli/option_types.py +++ b/core/dbt/cli/option_types.py @@ -1,11 +1,10 @@ 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 DbtConfigError, DbtValidationError +from dbt_common.exceptions import DbtValidationError from dbt_common.helper_types import WarnErrorOptions -from typing import Any, Dict class YAML(ParamType): @@ -44,28 +43,6 @@ 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.""" diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index 7cdbebf0328..42bab656dca 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -26,6 +26,7 @@ from dbt.clients.yaml_helper import load_yaml_text from dbt.adapters.contracts.connection import QueryComment from dbt.exceptions import ( + DbtConfigError, DbtProjectError, ProjectContractBrokenError, ProjectContractError, @@ -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, @@ -835,10 +837,18 @@ def read_project_flags(project_dir: str, profiles_dir: str) -> ProjectFlags: project_flags = profile_project_flags if project_flags is not None: + # if warn_error_options are set, handle collapsing `include` and `error` as well as + # collapsing `exclude` and `warn` + warn_error_options = project_flags.get("warn_error_options") + if warn_error_options: + exclusive_primary_alt_value_setting(warn_error_options, "include", "error") + exclusive_primary_alt_value_setting(warn_error_options, "exclude", "warn") + 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, DbtConfigError) 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 diff --git a/core/dbt/config/utils.py b/core/dbt/config/utils.py index bb5546cc743..daeac6bed40 100644 --- a/core/dbt/config/utils.py +++ b/core/dbt/config/utils.py @@ -5,7 +5,7 @@ from dbt_common.events.functions import fire_event from dbt.events.types import InvalidOptionYAML from dbt.exceptions import OptionNotYamlDictError -from dbt_common.exceptions import DbtValidationError +from dbt_common.exceptions import DbtConfigError, DbtValidationError def parse_cli_vars(var_string: str) -> Dict[str, Any]: @@ -23,3 +23,25 @@ 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: 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 diff --git a/tests/functional/configs/test_warn_error_options.py b/tests/functional/configs/test_warn_error_options.py index 3ab15974f72..2363fc469fd 100644 --- a/tests/functional/configs/test_warn_error_options.py +++ b/tests/functional/configs/test_warn_error_options.py @@ -175,3 +175,31 @@ def test_can_exclude_specific_event( 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) From 53d61f983a64a7b2e77b2aa064797654cd79fe4b Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Fri, 26 Apr 2024 14:40:15 -0700 Subject: [PATCH 12/17] Use `DbtWarnErrorOptionsError` 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. --- core/dbt/config/project.py | 4 ++-- core/dbt/config/utils.py | 6 +++--- core/dbt/exceptions.py | 4 ++++ 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index 42bab656dca..79c5a1e881c 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -26,8 +26,8 @@ from dbt.clients.yaml_helper import load_yaml_text from dbt.adapters.contracts.connection import QueryComment from dbt.exceptions import ( - DbtConfigError, DbtProjectError, + DbtWarnErrorOptionsError, ProjectContractBrokenError, ProjectContractError, DbtRuntimeError, @@ -846,7 +846,7 @@ def read_project_flags(project_dir: str, profiles_dir: str) -> ProjectFlags: ProjectFlags.validate(project_flags) return ProjectFlags.from_dict(project_flags) - except (DbtProjectError, DbtConfigError) as exc: + except (DbtProjectError, DbtWarnErrorOptionsError) as exc: # We don't want to eat the DbtProjectError for UserConfig to ProjectFlags or # DbtConfigError for warn_error_options munging raise exc diff --git a/core/dbt/config/utils.py b/core/dbt/config/utils.py index daeac6bed40..0bd9f1eb6a5 100644 --- a/core/dbt/config/utils.py +++ b/core/dbt/config/utils.py @@ -4,8 +4,8 @@ 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_common.exceptions import DbtConfigError, DbtValidationError +from dbt.exceptions import DbtWarnErrorOptionsError, OptionNotYamlDictError +from dbt_common.exceptions import DbtValidationError def parse_cli_vars(var_string: str) -> Dict[str, Any]: @@ -39,7 +39,7 @@ def exclusive_primary_alt_value_setting( alt_options = dictionary.get(alt) if primary_options and alt_options: - raise DbtConfigError( + raise DbtWarnErrorOptionsError( f"Only `{alt}` or `{primary}` can be specified in `warn_error_options`, not both" ) diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py index eb4d0b97276..a692fad5389 100644 --- a/core/dbt/exceptions.py +++ b/core/dbt/exceptions.py @@ -113,6 +113,10 @@ class DbtProfileError(DbtConfigError): pass +class DbtWarnErrorOptionsError(DbtConfigError): + pass + + class InvalidSelectorError(DbtRuntimeError): def __init__(self, name: str) -> None: self.name = name From 0e87a17ff64486d0b5e311934e0af35c38dc49a8 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Fri, 26 Apr 2024 23:32:28 -0500 Subject: [PATCH 13/17] Add changie documentation for 9644 features --- .changes/unreleased/Features-20240426-233126.yaml | 6 ++++++ .changes/unreleased/Features-20240426-233208.yaml | 6 ++++++ 2 files changed, 12 insertions(+) create mode 100644 .changes/unreleased/Features-20240426-233126.yaml create mode 100644 .changes/unreleased/Features-20240426-233208.yaml diff --git a/.changes/unreleased/Features-20240426-233126.yaml b/.changes/unreleased/Features-20240426-233126.yaml new file mode 100644 index 00000000000..37cd947f050 --- /dev/null +++ b/.changes/unreleased/Features-20240426-233126.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Abillity to `silence` warnings via `warn_error_options` +time: 2024-04-26T23:31:26.601057-05:00 +custom: + Author: QMalcolm + Issue: "9644" diff --git a/.changes/unreleased/Features-20240426-233208.yaml b/.changes/unreleased/Features-20240426-233208.yaml new file mode 100644 index 00000000000..c1cedc08ec3 --- /dev/null +++ b/.changes/unreleased/Features-20240426-233208.yaml @@ -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" From 49b66e10549190128a1a228a41a2d3f31c1f2ef1 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Mon, 29 Apr 2024 14:23:39 -0700 Subject: [PATCH 14/17] 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. --- tests/unit/config/test_utils.py | 41 +++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 tests/unit/config/test_utils.py diff --git a/tests/unit/config/test_utils.py b/tests/unit/config/test_utils.py new file mode 100644 index 00000000000..57ede0dfb00 --- /dev/null +++ b/tests/unit/config/test_utils.py @@ -0,0 +1,41 @@ +import pytest + +from dbt.config.utils import exclusive_primary_alt_value_setting +from dbt.exceptions import DbtWarnErrorOptionsError + + +class TestExclusivePrimaryAltValueSetting: + @pytest.fixture(scope="class") + def primary_key(self) -> str: + return "key_a" + + @pytest.fixture(scope="class") + def alt_key(self) -> str: + return "key_b" + + @pytest.fixture(scope="class") + def value(self) -> str: + return "I LIKE CATS" + + def test_primary_set(self, primary_key: str, alt_key: str, value: str): + test_dict = {primary_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) is None + + 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} + with pytest.raises(DbtWarnErrorOptionsError): + exclusive_primary_alt_value_setting(test_dict, primary_key, alt_key) + + def test_neither_primary_nor_alt_set(self, primary_key: str, alt_key: str): + test_dict = {} + 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 From e1c783945d7808708c472f15f38319c126eae2d9 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Tue, 30 Apr 2024 14:25:23 -0700 Subject: [PATCH 15/17] Fixup typo in changie doc `Features-20240426-233126.yaml` --- .changes/unreleased/Features-20240426-233126.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/unreleased/Features-20240426-233126.yaml b/.changes/unreleased/Features-20240426-233126.yaml index 37cd947f050..e726288e814 100644 --- a/.changes/unreleased/Features-20240426-233126.yaml +++ b/.changes/unreleased/Features-20240426-233126.yaml @@ -1,5 +1,5 @@ kind: Features -body: Abillity to `silence` warnings via `warn_error_options` +body: Ability to `silence` warnings via `warn_error_options` time: 2024-04-26T23:31:26.601057-05:00 custom: Author: QMalcolm From 0d960b27e71207f0893b307c021ab1b37b652863 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Tue, 30 Apr 2024 15:37:06 -0700 Subject: [PATCH 16/17] Update `exclusive_primary_alt_value_setting` to handle `None` dictionary --- core/dbt/config/project.py | 9 ++++----- core/dbt/config/utils.py | 7 +++++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index 79c5a1e881c..7309df3cbd6 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -837,12 +837,11 @@ def read_project_flags(project_dir: str, profiles_dir: str) -> ProjectFlags: project_flags = profile_project_flags if project_flags is not None: - # if warn_error_options are set, handle collapsing `include` and `error` as well as - # collapsing `exclude` and `warn` + # 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") - if warn_error_options: - exclusive_primary_alt_value_setting(warn_error_options, "include", "error") - exclusive_primary_alt_value_setting(warn_error_options, "exclude", "warn") + exclusive_primary_alt_value_setting(warn_error_options, "include", "error") + exclusive_primary_alt_value_setting(warn_error_options, "exclude", "warn") 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 0bd9f1eb6a5..b3a40af884a 100644 --- a/core/dbt/config/utils.py +++ b/core/dbt/config/utils.py @@ -1,4 +1,4 @@ -from typing import Any, Dict +from typing import Any, Dict, Optional from dbt.clients import yaml_helper @@ -26,7 +26,7 @@ def parse_cli_yaml_string(var_string: str, cli_option_name: str) -> Dict[str, An def exclusive_primary_alt_value_setting( - dictionary: Dict[str, Any], primary: str, alt: str + dictionary: Optional[Dict[str, Any]], primary: str, alt: str ) -> None: """Munges in place under the primary the options for the primary and alt values @@ -35,6 +35,9 @@ def exclusive_primary_alt_value_setting( 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) From 41e6d7cb76b5df9a7df432f461738070d68ef79e Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Tue, 30 Apr 2024 15:48:39 -0700 Subject: [PATCH 17/17] Update `exclusive_primary_alt_value_setting` to raise more generalized error --- core/dbt/cli/option_types.py | 8 ++++++-- core/dbt/config/project.py | 12 ++++++++---- core/dbt/config/utils.py | 12 ++++++++---- core/dbt/exceptions.py | 2 +- tests/unit/config/test_utils.py | 4 ++-- 5 files changed, 25 insertions(+), 13 deletions(-) diff --git a/core/dbt/cli/option_types.py b/core/dbt/cli/option_types.py index ef04e348260..544b09bee2a 100644 --- a/core/dbt/cli/option_types.py +++ b/core/dbt/cli/option_types.py @@ -51,8 +51,12 @@ 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") + 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", []), diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index 7309df3cbd6..52b20e5179d 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -27,7 +27,7 @@ from dbt.adapters.contracts.connection import QueryComment from dbt.exceptions import ( DbtProjectError, - DbtWarnErrorOptionsError, + DbtExclusivePropertyUseError, ProjectContractBrokenError, ProjectContractError, DbtRuntimeError, @@ -840,12 +840,16 @@ def read_project_flags(project_dir: str, profiles_dir: str) -> ProjectFlags: # 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") - exclusive_primary_alt_value_setting(warn_error_options, "exclude", "warn") + 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, DbtWarnErrorOptionsError) as exc: + 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 diff --git a/core/dbt/config/utils.py b/core/dbt/config/utils.py index b3a40af884a..f94e5dce0ad 100644 --- a/core/dbt/config/utils.py +++ b/core/dbt/config/utils.py @@ -4,7 +4,7 @@ from dbt.clients import yaml_helper from dbt_common.events.functions import fire_event from dbt.events.types import InvalidOptionYAML -from dbt.exceptions import DbtWarnErrorOptionsError, OptionNotYamlDictError +from dbt.exceptions import DbtExclusivePropertyUseError, OptionNotYamlDictError from dbt_common.exceptions import DbtValidationError @@ -26,7 +26,10 @@ def parse_cli_yaml_string(var_string: str, cli_option_name: str) -> Dict[str, An def exclusive_primary_alt_value_setting( - dictionary: Optional[Dict[str, Any]], primary: str, alt: str + 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 @@ -42,8 +45,9 @@ def exclusive_primary_alt_value_setting( alt_options = dictionary.get(alt) if primary_options and alt_options: - raise DbtWarnErrorOptionsError( - f"Only `{alt}` or `{primary}` can be specified in `warn_error_options`, not both" + 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: diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py index a692fad5389..ea44f0c055c 100644 --- a/core/dbt/exceptions.py +++ b/core/dbt/exceptions.py @@ -113,7 +113,7 @@ class DbtProfileError(DbtConfigError): pass -class DbtWarnErrorOptionsError(DbtConfigError): +class DbtExclusivePropertyUseError(DbtConfigError): pass diff --git a/tests/unit/config/test_utils.py b/tests/unit/config/test_utils.py index 57ede0dfb00..d2d7f99499d 100644 --- a/tests/unit/config/test_utils.py +++ b/tests/unit/config/test_utils.py @@ -1,7 +1,7 @@ import pytest from dbt.config.utils import exclusive_primary_alt_value_setting -from dbt.exceptions import DbtWarnErrorOptionsError +from dbt.exceptions import DbtExclusivePropertyUseError class TestExclusivePrimaryAltValueSetting: @@ -31,7 +31,7 @@ def test_alt_set(self, primary_key: str, alt_key: str, value: str): def test_primary_and_alt_set(self, primary_key: str, alt_key: str, value: str): test_dict = {primary_key: value, alt_key: value} - with pytest.raises(DbtWarnErrorOptionsError): + with pytest.raises(DbtExclusivePropertyUseError): exclusive_primary_alt_value_setting(test_dict, primary_key, alt_key) def test_neither_primary_nor_alt_set(self, primary_key: str, alt_key: str):