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 authored Apr 22, 2024
1 parent 5c8a4ab commit 0290cf7
Show file tree
Hide file tree
Showing 10 changed files with 730 additions and 578 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"
28 changes: 25 additions & 3 deletions core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
)
from typing_extensions import Protocol

from dbt import deprecations
from dbt import tracking
from dbt.contracts.graph.nodes import (
BaseNode,
Expand Down Expand Up @@ -570,11 +571,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 @@ -930,7 +935,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 @@ -118,6 +118,11 @@ def show(self, *args, **kwargs) -> None:
active_deprecations.add(self.name)


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 @@ -157,6 +162,7 @@ def warn(name, *args, **kwargs):
CollectFreshnessReturnSignature(),
TestsConfigDeprecation(),
ProjectFlagsMovedDeprecation(),
PackageMaterializationOverrideDeprecation(),
]

deprecations: Dict[str, DBTDeprecation] = {d.name: d for d in deprecations_list}
Expand Down
11 changes: 11 additions & 0 deletions core/dbt/events/core_types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,17 @@ message TotalModelNamesWithSpacesDeprecation {
string level = 3;
}

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

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

message TotalModelNamesWithSpacesDeprecationMsg {
CoreEventInfo info = 1;
TotalModelNamesWithSpacesDeprecation data = 2;
Expand Down
1,146 changes: 575 additions & 571 deletions core/dbt/events/core_types_pb2.py

Large diffs are not rendered by default.

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


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
2 changes: 1 addition & 1 deletion tests/functional/deprecations/test_deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def test_exposure_name_fail(self, project):
assert expected_msg in exc_str


class TestPrjectFlagsMovedDeprecation:
class TestProjectFlagsMovedDeprecation:
@pytest.fixture(scope="class")
def profiles_config_update(self):
return {
Expand Down
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 @@ -158,6 +158,9 @@ def test_event_codes(self):
core_types.TotalModelNamesWithSpacesDeprecation(
count_invalid_names=1, show_debug_hint=True, level=""
),
core_types.PackageMaterializationOverrideDeprecation(
package_name="my_package", materialization_name="view"
),
# E - DB Adapter ======================
adapter_types.AdapterEventDebug(),
adapter_types.AdapterEventInfo(),
Expand Down

0 comments on commit 0290cf7

Please sign in to comment.