Skip to content

Commit

Permalink
Add flags.deprecate_package_materialization_builtin_override (#9956)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichelleArk authored Apr 23, 2024
1 parent 1014a6d commit 2d33655
Show file tree
Hide file tree
Showing 7 changed files with 312 additions and 10 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Features-20240422-173703.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: Add require_explicit_package_overrides_for_builtin_materializations to dbt_project.yml flags, which can be used to opt-out of overriding built-in materializations from packages
time: 2024-04-22T17:37:03.892268-04:00
custom:
Author: michelleark
Issue: "10007"
2 changes: 1 addition & 1 deletion core/dbt/cli/flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ def set_common_global_flags(self):
# This is here to prevent mypy from complaining about all of the
# attributes which we added dynamically.
def __getattr__(self, name: str) -> Any:
return super().__get_attribute__(name) # type: ignore
return super().__getattribute__(name) # type: ignore


CommandParams = List[str]
Expand Down
37 changes: 30 additions & 7 deletions core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -571,11 +571,25 @@ def __lt__(self, other: object) -> bool:


class CandidateList(List[M]):
def last_candidate(self) -> Optional[MacroCandidate]:
def last_candidate(
self, valid_localities: Optional[List[Locality]] = None
) -> Optional[MacroCandidate]:
"""
Obtain the last (highest precedence) MacroCandidate from the CandidateList of any locality in valid_localities.
If valid_localities is not specified, return the last MacroCandidate of any locality.
"""
if not self:
return None
self.sort()
return self[-1]

if valid_localities is None:
return self[-1]

for candidate in reversed(self):
if candidate.locality in valid_localities:
return candidate

return None

def last(self) -> Optional[Macro]:
last_candidate = self.last_candidate()
Expand Down Expand Up @@ -946,11 +960,20 @@ def find_materialization_macro_by_name(
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,
)
# preserve legacy behaviour - allow materialization override
if (
get_flags().require_explicit_package_overrides_for_builtin_materializations
is False
):
deprecations.warn(
"package-materialization-override",
package_name=materialization_candidate.macro.package_name,
materialization_name=materialization_name,
)
else:
materialization_candidate = candidates.last_candidate(
valid_localities=[Locality.Core, Locality.Root]
)

return materialization_candidate.macro if materialization_candidate else None

Expand Down
2 changes: 2 additions & 0 deletions core/dbt/contracts/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ class ProjectFlags(ExtensibleDbtClassMixin):
partial_parse: Optional[bool] = None
populate_cache: Optional[bool] = None
printer_width: Optional[int] = None
require_explicit_package_overrides_for_builtin_materializations: bool = False
send_anonymous_usage_stats: bool = DEFAULT_SEND_ANONYMOUS_USAGE_STATS
source_freshness_run_project_hooks: bool = False
static_parser: Optional[bool] = None
Expand All @@ -324,6 +325,7 @@ def project_only_flags(self) -> Dict[str, Any]:
return {
"source_freshness_run_project_hooks": self.source_freshness_run_project_hooks,
"allow_spaces_in_model_names": self.allow_spaces_in_model_names,
"require_explicit_package_overrides_for_builtin_materializations": self.require_explicit_package_overrides_for_builtin_materializations,
}


Expand Down
2 changes: 1 addition & 1 deletion core/dbt/events/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ 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."
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#require_explicit_package_overrides_for_builtin_materializations for detailed documentation and suggested workarounds."

return line_wrap_message(warning_tag(description))

Expand Down
96 changes: 96 additions & 0 deletions tests/functional/materializations/test_custom_materialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,56 @@ def test_adapter_dependency(self, project, override_view_adapter_dep, set_up_dep
assert deprecations.active_deprecations == {"package-materialization-override"}


class TestOverrideAdapterDependencyDeprecated:
# 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"}]}

@pytest.fixture(scope="class")
def project_config_update(self):
return {
"flags": {
"require_explicit_package_overrides_for_builtin_materializations": True,
},
}

def test_adapter_dependency_deprecate_overrides(
self, project, override_view_adapter_dep, set_up_deprecations
):
run_dbt(["deps"])
# this should pass because the override is buggy and unused
run_dbt(["run"])

# no deprecation warning -- flag used correctly
assert deprecations.active_deprecations == set()


class TestOverrideAdapterDependencyLegacy:
# 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"}]}

@pytest.fixture(scope="class")
def project_config_update(self):
return {
"flags": {
"require_explicit_package_overrides_for_builtin_materializations": False,
},
}

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):
Expand All @@ -58,6 +108,52 @@ def test_default_dependency(self, project, override_view_default_dep, set_up_dep
assert deprecations.active_deprecations == {"package-materialization-override"}


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

@pytest.fixture(scope="class")
def project_config_update(self):
return {
"flags": {
"require_explicit_package_overrides_for_builtin_materializations": True,
},
}

def test_default_dependency_deprecated(
self, project, override_view_default_dep, set_up_deprecations
):
run_dbt(["deps"])
# this should pass because the override is buggy and unused
run_dbt(["run"])

# overriding a built-in materialization from package is deprecated
assert deprecations.active_deprecations == set()


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

@pytest.fixture(scope="class")
def project_config_update(self):
return {
"flags": {
"require_explicit_package_overrides_for_builtin_materializations": False,
},
}

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()) }}
Expand Down
Loading

0 comments on commit 2d33655

Please sign in to comment.