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

deprecations.buffer: respect --quiet and --warn-error-options for deprecations #10534

Merged
merged 4 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/Fixes-20240806-194843.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: respect --quiet and --warn-error-options for flag deprecations
time: 2024-08-06T19:48:43.399453-04:00
custom:
Author: michelleark
Issue: "10105"
4 changes: 3 additions & 1 deletion core/dbt/cli/flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from dbt.cli.types import Command as CliCommand
from dbt.config.project import read_project_flags
from dbt.contracts.project import ProjectFlags
from dbt.deprecations import renamed_env_var
from dbt.deprecations import fire_buffered_deprecations, renamed_env_var
from dbt.events import ALL_EVENT_NAMES
from dbt_common import ui
from dbt_common.clients import jinja
Expand Down Expand Up @@ -355,6 +355,8 @@ def fire_deprecations(self):
# not get pickled when written to disk as json.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to refactor the deprecated_env_var_warnings mechanism above to use the new deprecations.buffer pattern but will defer that to a follow-up PR

object.__delattr__(self, "deprecated_env_var_warnings")

fire_buffered_deprecations()

@classmethod
def from_dict(cls, command: CliCommand, args_dict: Dict[str, Any]) -> "Flags":
command_arg_list = command_params(command, args_dict)
Expand Down
4 changes: 2 additions & 2 deletions core/dbt/config/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -821,8 +821,8 @@

if profile_project_flags:
# This can't use WARN_ERROR or WARN_ERROR_OPTIONS because they're in
# the config that we're loading. Uses special "warn" method.
deprecations.warn("project-flags-moved")
# the config that we're loading. Uses special "buffer" method and fired after flags are initialized in preflight.
deprecations.buffer("project-flags-moved")

Check warning on line 825 in core/dbt/config/project.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/config/project.py#L825

Added line #L825 was not covered by tests
project_flags = profile_project_flags

if project_flags is not None:
Expand Down
27 changes: 16 additions & 11 deletions core/dbt/deprecations.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import abc
from typing import ClassVar, Dict, List, Optional, Set
from typing import Callable, ClassVar, Dict, List, Optional, Set

import dbt.tracking
from dbt.events import types as core_types
from dbt_common.events.functions import fire_event, warn_or_error
from dbt_common.events.functions import warn_or_error


class DBTDeprecation:
Expand Down Expand Up @@ -107,15 +107,6 @@ class ProjectFlagsMovedDeprecation(DBTDeprecation):
_name = "project-flags-moved"
_event = "ProjectFlagsMovedDeprecation"

def show(self, *args, **kwargs) -> None:
if self.name not in active_deprecations:
event = self.event(**kwargs)
# We can't do warn_or_error because the ProjectFlags
# is where that is set up and we're just reading it.
fire_event(event)
self.track_deprecation_warn()
active_deprecations.add(self.name)


class PackageMaterializationOverrideDeprecation(DBTDeprecation):
_name = "package-materialization-override"
Expand Down Expand Up @@ -155,6 +146,13 @@ def warn(name, *args, **kwargs):
deprecations[name].show(*args, **kwargs)


def buffer(name: str, *args, **kwargs):
def show_callback():
deprecations[name].show(*args, **kwargs)

buffered_deprecations.append(show_callback)


# these are globally available
# since modules are only imported once, active_deprecations is a singleton

Expand All @@ -178,6 +176,13 @@ def warn(name, *args, **kwargs):

deprecations: Dict[str, DBTDeprecation] = {d.name: d for d in deprecations_list}

buffered_deprecations: List[Callable] = []


def reset_deprecations():
active_deprecations.clear()


def fire_buffered_deprecations():
[dep_fn() for dep_fn in buffered_deprecations]
buffered_deprecations.clear()
48 changes: 44 additions & 4 deletions tests/functional/deprecations/test_deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

import dbt_common
from dbt import deprecations
from dbt.tests.util import run_dbt, write_file
from dbt.tests.util import run_dbt, run_dbt_and_capture, write_file
from dbt_common.exceptions import EventCompilationError
from tests.functional.deprecations.fixtures import (
bad_name_yaml,
models_trivial__model_sql,
Expand Down Expand Up @@ -143,6 +144,45 @@ def models(self):
def test_profile_config_deprecation(self, project):
deprecations.reset_deprecations()
assert deprecations.active_deprecations == set()
run_dbt(["parse"])
expected = {"project-flags-moved"}
assert expected == deprecations.active_deprecations

_, logs = run_dbt_and_capture(["parse"])

assert (
"User config should be moved from the 'config' key in profiles.yml to the 'flags' key in dbt_project.yml."
in logs
)
assert deprecations.active_deprecations == {"project-flags-moved"}


class TestProjectFlagsMovedDeprecationQuiet(TestProjectFlagsMovedDeprecation):
def test_profile_config_deprecation(self, project):
deprecations.reset_deprecations()
assert deprecations.active_deprecations == set()

_, logs = run_dbt_and_capture(["--quiet", "parse"])

assert (
"User config should be moved from the 'config' key in profiles.yml to the 'flags' key in dbt_project.yml."
not in logs
)
assert deprecations.active_deprecations == {"project-flags-moved"}


class TestProjectFlagsMovedDeprecationWarnErrorOptions(TestProjectFlagsMovedDeprecation):
def test_profile_config_deprecation(self, project):
deprecations.reset_deprecations()
with pytest.raises(EventCompilationError):
run_dbt(["--warn-error-options", "{'include': 'all'}", "parse"])

with pytest.raises(EventCompilationError):
run_dbt(
["--warn-error-options", "{'include': ['ProjectFlagsMovedDeprecation']}", "parse"]
)

_, logs = run_dbt_and_capture(
["--warn-error-options", "{'silence': ['ProjectFlagsMovedDeprecation']}", "parse"]
)
assert (
"User config should be moved from the 'config' key in profiles.yml to the 'flags' key in dbt_project.yml."
not in logs
)
36 changes: 36 additions & 0 deletions tests/unit/test_deprecations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import pytest

import dbt.deprecations as deprecations


@pytest.fixture(scope="function")
def active_deprecations():
assert not deprecations.active_deprecations

yield deprecations.active_deprecations

deprecations.reset_deprecations()


@pytest.fixture(scope="function")
def buffered_deprecations():
assert not deprecations.buffered_deprecations

yield deprecations.buffered_deprecations

deprecations.buffered_deprecations.clear()


def test_buffer_deprecation(active_deprecations, buffered_deprecations):
deprecations.buffer("project-flags-moved")

assert active_deprecations == set()
assert len(buffered_deprecations) == 1


def test_fire_buffered_deprecations(active_deprecations, buffered_deprecations):
deprecations.buffer("project-flags-moved")
deprecations.fire_buffered_deprecations()

assert active_deprecations == set(["project-flags-moved"])
assert len(buffered_deprecations) == 0
Loading