-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
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 3292c3a
Raise minimum version of dbt_common to 1.0.2
QMalcolm f892f3d
Add ability to silence warnings from `warn_error_options` CLI arguments
QMalcolm 4a94d6a
Add `flush` to `EventCatcher` test util, and use in `test_warn_error_…
QMalcolm e02df2d
Add tests to `TestWarnErrorOptionsFromCLI` for `include` and `exclude`
QMalcolm 9f05e1f
Refactor `runner` and `catcher` into fixtures for `TestWarnErrorOptio…
QMalcolm c52d653
Test support for setting `silence` of `warn_error_options` in `dbt_pr…
QMalcolm cf72bf1
Add tests to `TestWarnErrorOptionsFromProject` for `include` and `exc…
QMalcolm 4c7e04b
Abstract repetitive assertion logic in `test_warn_error_options.py`
QMalcolm ebf9106
Update `warn_error_options` in CLI args to support `error` and `warn`…
QMalcolm 3eb4e0c
Update `warn_error_options` in Project flags to support `error` and `…
QMalcolm 53d61f9
Use `DbtWarnErrorOptionsError` in `exclusive_primary_alt_value_settin…
QMalcolm 0e87a17
Add changie documentation for 9644 features
QMalcolm 49b66e1
Add unit tests for `exclusive_primary_alt_value_setting` method
QMalcolm e1c7839
Fixup typo in changie doc `Features-20240426-233126.yaml`
QMalcolm 0d960b2
Update `exclusive_primary_alt_value_setting` to handle `None` dictionary
QMalcolm 41e6d7c
Update `exclusive_primary_alt_value_setting` to raise more generalize…
QMalcolm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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) |
16 changes: 3 additions & 13 deletions
16
tests/functional/manifest_validations/test_check_for_spaces_in_model_names.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 classWarnErrorOptions
is new, and got released in dbt-common1.0.2
. If we don't bump the minimum, then it would be possible for us to get dbt-common1.0.1
which doesn't support silence, and we'd break at run time. Here is the work in dbt-common which added the functionalityThere was a problem hiding this comment.
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.