From 11ee2b9c4233c2c8d28a15ff63a98a4dcc72dc7f Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Tue, 25 Jun 2024 13:53:18 -0700 Subject: [PATCH] Fix setting `silence` of `warn_error_options` via `dbt_project.yaml` flags (#10359) * Fix `test_can_silence` tests in `test_warn_error_options.py` to ensure silencing We're fixing an issue wherein `silence` specifications in the `dbt_project.yaml` weren't being respected. This was odd since we had tests specifically for this. It turns out the tests were broken. Essentially the warning was instead being raised as an error due to `include: 'all'`. Then because it was being raised as an error, the event wasn't going through the logger. We were only asserting in these tests that the silenced event wasn't going through the logger (which it wasn't) so everything "appeared" to be working. Unfortunately everything wasn't actually working. This is now highlighted because `test_warn_error_options::TestWarnErrorOptionsFromProject:test_can_silence` is now failing with this commit. * Fix setting `warn_error_options` via `dbt_project.yaml` flags. Back when I did the work for #10058 (specifically c52d6531) I thought that the `warn_error_options` would automatically be converted from the yaml to the `WarnErrorOptions` object as we were building the `ProjectFlags` object, which holds `warn_error_options`, via `ProjectFlags.from_dict`. And I thought this was validated by the `test_can_silence` test added in c52d6531. However, there were two problems: 1. The definition of `warn_error_options` on `PrjectFlags` is a dict, not a `WarnErrorOptions` object 2. The `test_can_silence` test was broken, and not testing what I thought The quick fix (this commit) is to ensure `silence` is passed to `WarnErrorOptions` instantiation in `dbt.cli.flags.convert_config`. The better fix would be to make the `warn_error_options` of `ProjectFlags` a `WarnErrorOptions` object instead of a dict. However, to do this we first need to update dbt-common's `WarnErrorOptions` definition to default `include` to an empty list. Doing so would allow us to get rid of `convert_config` entirely. --- .changes/unreleased/Fixes-20240624-171729.yaml | 6 ++++++ core/dbt/cli/flags.py | 1 + tests/functional/configs/test_warn_error_options.py | 12 +++++------- 3 files changed, 12 insertions(+), 7 deletions(-) create mode 100644 .changes/unreleased/Fixes-20240624-171729.yaml diff --git a/.changes/unreleased/Fixes-20240624-171729.yaml b/.changes/unreleased/Fixes-20240624-171729.yaml new file mode 100644 index 00000000000..f121ac5aa8b --- /dev/null +++ b/.changes/unreleased/Fixes-20240624-171729.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Fix setting `silence` of `warn_error_options` via `dbt_project.yaml` flags +time: 2024-06-24T17:17:29.464865-07:00 +custom: + Author: QMalcolm + Issue: "10160" diff --git a/core/dbt/cli/flags.py b/core/dbt/cli/flags.py index a74172484f3..33c2e519819 100644 --- a/core/dbt/cli/flags.py +++ b/core/dbt/cli/flags.py @@ -57,6 +57,7 @@ def convert_config(config_name, config_value): ret = WarnErrorOptions( include=config_value.get("include", []), exclude=config_value.get("exclude", []), + silence=config_value.get("silence", []), valid_error_names=ALL_EVENT_NAMES, ) return ret diff --git a/tests/functional/configs/test_warn_error_options.py b/tests/functional/configs/test_warn_error_options.py index d8e68fcf11d..1808d8f26f0 100644 --- a/tests/functional/configs/test_warn_error_options.py +++ b/tests/functional/configs/test_warn_error_options.py @@ -53,9 +53,8 @@ def test_can_silence(self, project, catcher: EventCatcher, runner: dbtRunner) -> assert_deprecation_warning(result, catcher) catcher.flush() - runner.invoke( - ["run", "--warn-error-options", "{'include': 'all', 'silence': ['DeprecatedModel']}"] - ) + result = runner.invoke(["run", "--warn-error-options", "{'silence': ['DeprecatedModel']}"]) + assert result.success assert len(catcher.caught_events) == 0 def test_can_raise_warning_to_error( @@ -131,13 +130,12 @@ def test_can_silence( result = runner.invoke(["run"]) assert_deprecation_warning(result, catcher) - silence_options = { - "flags": {"warn_error_options": {"include": "all", "silence": ["DeprecatedModel"]}} - } + silence_options = {"flags": {"warn_error_options": {"silence": ["DeprecatedModel"]}}} update_config_file(silence_options, project_root, "dbt_project.yml") catcher.flush() - runner.invoke(["run"]) + result = runner.invoke(["run"]) + assert result.success assert len(catcher.caught_events) == 0 def test_can_raise_warning_to_error(