Skip to content

Commit

Permalink
deprecate materialization overrides from imported packages (#9971)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichelleArk committed Apr 22, 2024
1 parent 27f4d0f commit 747f7e3
Show file tree
Hide file tree
Showing 9 changed files with 1,034 additions and 883 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Under the Hood-20240418-172528.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Under the Hood
body: Raise deprecation warning if installed package overrides built-in materialization
time: 2024-04-18T17:25:28.37886-04:00
custom:
Author: michelleark
Issue: "9971"
30 changes: 25 additions & 5 deletions core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
)
from typing_extensions import Protocol
from uuid import UUID

from dbt.contracts.graph.nodes import (
BaseNode,
Documentation,
Expand Down Expand Up @@ -59,7 +58,7 @@
from dbt.events.contextvars import get_node_info
from dbt.node_types import NodeType, AccessType
from dbt.flags import get_flags, MP_CONTEXT
from dbt import tracking
from dbt import tracking, deprecations
import dbt.utils


Expand Down Expand Up @@ -562,11 +561,15 @@ def __lt__(self, other: object) -> bool:


class CandidateList(List[M]):
def last(self) -> Optional[Macro]:
def last_candidate(self) -> Optional[MacroCandidate]:
if not self:
return None
self.sort()
return self[-1].macro
return self[-1]

def last(self) -> Optional[Macro]:
last_candidate = self.last_candidate()
return last_candidate.macro if last_candidate is not None else None


def _get_locality(macro: Macro, root_project_name: str, internal_packages: Set[str]) -> Locality:
Expand Down Expand Up @@ -850,7 +853,24 @@ def find_materialization_macro_by_name(
for specificity, atype in enumerate(self._get_parent_adapter_types(adapter_type))
)
)
return candidates.last()
core_candidates = [
candidate for candidate in candidates if candidate.locality == Locality.Core
]

materialization_candidate = candidates.last_candidate()
# If an imported materialization macro was found that also had a core candidate, fire a deprecation
if (
materialization_candidate is not None
and materialization_candidate.locality == Locality.Imported
and core_candidates
):
deprecations.warn(
"package-materialization-override",
package_name=materialization_candidate.macro.package_name,
materialization_name=materialization_name,
)

return materialization_candidate.macro if materialization_candidate else None

def get_resource_fqns(self) -> Mapping[str, PathSet]:
resource_fqns: Dict[str, Set[Tuple[str, ...]]] = {}
Expand Down
6 changes: 6 additions & 0 deletions core/dbt/deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ class CollectFreshnessReturnSignature(DBTDeprecation):
_event = "CollectFreshnessReturnSignature"


class PackageMaterializationOverrideDeprecation(DBTDeprecation):
_name = "package-materialization-override"
_event = "PackageMaterializationOverrideDeprecation"


def renamed_env_var(old_name: str, new_name: str):
class EnvironmentVariableRenamed(DBTDeprecation):
_name = f"environment-variable-renamed:{old_name}"
Expand Down Expand Up @@ -134,6 +139,7 @@ def warn(name, *args, **kwargs):
ConfigLogPathDeprecation(),
ConfigTargetPathDeprecation(),
CollectFreshnessReturnSignature(),
PackageMaterializationOverrideDeprecation(),
]

deprecations: Dict[str, DBTDeprecation] = {d.name: d for d in deprecations_list}
Expand Down
11 changes: 11 additions & 0 deletions core/dbt/events/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,17 @@ message CollectFreshnessReturnSignatureMsg {
CollectFreshnessReturnSignature data = 2;
}

// D016
message PackageMaterializationOverrideDeprecation {
string package_name = 1;
string materialization_name = 2;
}

message PackageMaterializationOverrideDeprecationMsg {
EventInfo info = 1;
PackageMaterializationOverrideDeprecation data = 2;
}

// E - DB Adapter

// E001
Expand Down
10 changes: 10 additions & 0 deletions core/dbt/events/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,16 @@ def message(self) -> str:
return line_wrap_message(warning_tag(msg))


class PackageMaterializationOverrideDeprecation(WarnLevel):
def code(self) -> str:
return "D016"

def message(self) -> str:
description = f"Installed package '{self.package_name}' is overriding the built-in materialization '{self.materialization_name}'. Overrides of built-in materializations from installed packages will be deprecated in future versions of dbt. Please refer to https://docs.getdbt.com/reference/global-configs/legacy-behaviors#deprecate_package_materialization_builtin_override for detailed documentation and suggested workarounds."

return line_wrap_message(warning_tag(description))


# =======================================================
# I - Project parsing
# =======================================================
Expand Down
1,755 changes: 880 additions & 875 deletions core/dbt/events/types_pb2.py

Large diffs are not rendered by default.

24 changes: 24 additions & 0 deletions tests/functional/materializations/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,21 @@
{%- endmaterialization -%}
"""

custom_materialization_dep__dbt_project_yml = """
name: custom_materialization_default
macro-paths: ['macros']
"""

custom_materialization_sql = """
{% materialization custom_materialization, default %}
{%- set target_relation = this.incorporate(type='table') %}
{% call statement('main') -%}
select 1 as column1
{%- endcall %}
{{ return({'relations': [target_relation]}) }}
{% endmaterialization %}
"""


@pytest.fixture(scope="class")
def override_view_adapter_pass_dep(project_root):
Expand Down Expand Up @@ -368,3 +383,12 @@ def override_view_return_no_relation(project_root):
},
}
write_project_files(project_root, "override-view-return-no-relation", files)


@pytest.fixture(scope="class")
def custom_materialization_dep(project_root):
files = {
"dbt_project.yml": custom_materialization_dep__dbt_project_yml,
"macros": {"custom_materialization.sql": custom_materialization_sql},
}
write_project_files(project_root, "custom-materialization-dep", files)
72 changes: 69 additions & 3 deletions tests/functional/materializations/test_custom_materialization.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import pytest

from dbt.tests.util import run_dbt

from dbt import deprecations

models__model_sql = """
{{ config(materialized='view') }}
Expand All @@ -10,34 +10,100 @@
"""


models_custom_materialization__model_sql = """
{{ config(materialized='custom_materialization') }}
select 1 as id
"""


@pytest.fixture(scope="class")
def models():
return {"model.sql": models__model_sql}


@pytest.fixture(scope="class")
def set_up_deprecations():
deprecations.reset_deprecations()
assert deprecations.active_deprecations == set()


class TestOverrideAdapterDependency:
# make sure that if there's a dependency with an adapter-specific
# materialization, we honor that materialization
@pytest.fixture(scope="class")
def packages(self):
return {"packages": [{"local": "override-view-adapter-dep"}]}

def test_adapter_dependency(self, project, override_view_adapter_dep):
def test_adapter_dependency(self, project, override_view_adapter_dep, set_up_deprecations):
run_dbt(["deps"])
# this should error because the override is buggy
run_dbt(["run"], expect_pass=False)

# overriding a built-in materialization scoped to adapter from package is deprecated
assert deprecations.active_deprecations == {"package-materialization-override"}


class TestOverrideDefaultDependency:
@pytest.fixture(scope="class")
def packages(self):
return {"packages": [{"local": "override-view-default-dep"}]}

def test_default_dependency(self, project, override_view_default_dep):
def test_default_dependency(self, project, override_view_default_dep, set_up_deprecations):
run_dbt(["deps"])
# this should error because the override is buggy
run_dbt(["run"], expect_pass=False)

# overriding a built-in materialization from package is deprecated
assert deprecations.active_deprecations == {"package-materialization-override"}


root_view_override_macro = """
{% materialization view, default %}
{{ return(view_default_override.materialization_view_default()) }}
{% endmaterialization %}
"""


class TestOverrideDefaultDependencyRootOverride:
@pytest.fixture(scope="class")
def packages(self):
return {"packages": [{"local": "override-view-default-dep"}]}

@pytest.fixture(scope="class")
def macros(self):
return {"my_view.sql": root_view_override_macro}

def test_default_dependency_with_root_override(
self, project, override_view_default_dep, set_up_deprecations
):
run_dbt(["deps"])
# this should error because the override is buggy
run_dbt(["run"], expect_pass=False)

# using an package-overriden built-in materialization in a root matereialization is _not_ deprecated
assert deprecations.active_deprecations == set()


class TestCustomMaterializationDependency:
@pytest.fixture(scope="class")
def models(self):
return {"model.sql": models_custom_materialization__model_sql}

@pytest.fixture(scope="class")
def packages(self):
return {"packages": [{"local": "custom-materialization-dep"}]}

def test_custom_materialization_deopendency(
self, project, custom_materialization_dep, set_up_deprecations
):
run_dbt(["deps"])
# custom materilization is valid
run_dbt(["run"])

# using a custom materialization is from an installed package is _not_ deprecated
assert deprecations.active_deprecations == set()


class TestOverrideAdapterDependencyPassing:
@pytest.fixture(scope="class")
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ def test_event_codes(self):
types.ConfigLogPathDeprecation(deprecated_path=""),
types.ConfigTargetPathDeprecation(deprecated_path=""),
types.CollectFreshnessReturnSignature(),
types.PackageMaterializationOverrideDeprecation(
package_name="my_package", materialization_name="view"
),
# E - DB Adapter ======================
types.AdapterEventDebug(),
types.AdapterEventInfo(),
Expand Down

0 comments on commit 747f7e3

Please sign in to comment.