Add flags.deprecate_package_materialization_builtin_override (#9956)

.changes/unreleased/Features-20240422-173703.yaml

@@ -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"

core/dbt/contracts/graph/manifest.py

@@ -617,11 +617,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()
@@ -930,11 +944,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
 

core/dbt/contracts/project.py

@@ -295,6 +295,7 @@ class ProjectFlags(ExtensibleDbtClassMixin, Replaceable):
  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
  static_parser: Optional[bool] = None
  use_colors: Optional[bool] = None
@@ -307,7 +308,9 @@ class ProjectFlags(ExtensibleDbtClassMixin, Replaceable):
 
  @property
  def project_only_flags(self) -> Dict[str, Any]:
- return {}
+ return {
+ "require_explicit_package_overrides_for_builtin_materializations": self.require_explicit_package_overrides_for_builtin_materializations,
+ }
 
 
 @dataclass

tests/functional/materializations/test_custom_materialization.py

@@ -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):
@@ -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()) }}

tests/unit/test_manifest.py

@@ -1239,7 +1239,7 @@ def test_find_generate_macros_by_name(macros, expectations):
 FindMaterializationSpec = namedtuple("FindMaterializationSpec", "macros,adapter_type,expected")
 
 
-def _materialization_parameter_sets():
+def _materialization_parameter_sets_legacy():
  # inject the plugins used for materialization parameter tests
  with mock.patch("dbt.adapters.base.plugin.project_name_from_path") as get_name:
  get_name.return_value = "foo"
@@ -1386,12 +1386,187 @@ def id_mat(arg):
  return "_".join(arg)
 
 
+@pytest.mark.parametrize(
+ "macros,adapter_type,expected",
+ _materialization_parameter_sets_legacy(),
+ ids=id_mat,
+)
+def test_find_materialization_by_name_legacy(macros, adapter_type, expected):
+ set_from_args(
+ Namespace(
+ SEND_ANONYMOUS_USAGE_STATS=False,
+ REQUIRE_EXPLICIT_PACKAGE_OVERRIDES_FOR_BUILTIN_MATERIALIZATIONS=False,
+ ),
+ None,
+ )
+
+ manifest = make_manifest(macros=macros)
+ result = manifest.find_materialization_macro_by_name(
+ project_name="root",
+ materialization_name="my_materialization",
+ adapter_type=adapter_type,
+ )
+ if expected is None:
+ assert result is expected
+ else:
+ expected_package, expected_adapter_type = expected
+ assert result.adapter_type == expected_adapter_type
+ assert result.package_name == expected_package
+
+
+def _materialization_parameter_sets():
+ # inject the plugins used for materialization parameter tests
+ with mock.patch("dbt.adapters.base.plugin.project_name_from_path") as get_name:
+ get_name.return_value = "foo"
+ FooPlugin = AdapterPlugin(
+ adapter=mock.MagicMock(),
+ credentials=mock.MagicMock(),
+ include_path="/path/to/root/plugin",
+ )
+ FooPlugin.adapter.type.return_value = "foo"
+ inject_plugin(FooPlugin)
+
+ BarPlugin = AdapterPlugin(
+ adapter=mock.MagicMock(),
+ credentials=mock.MagicMock(),
+ include_path="/path/to/root/plugin",
+ dependencies=["foo"],
+ )
+ BarPlugin.adapter.type.return_value = "bar"
+ inject_plugin(BarPlugin)
+
+ sets = [
+ FindMaterializationSpec(macros=[], adapter_type="foo", expected=None),
+ ]
+
+ # default only, each project
+ sets.extend(
+ FindMaterializationSpec(
+ macros=[MockMaterialization(project, adapter_type=None)],
+ adapter_type="foo",
+ expected=(project, "default"),
+ )
+ for project in ["root", "dep", "dbt"]
+ )
+
+ # other type only, each project
+ sets.extend(
+ FindMaterializationSpec(
+ macros=[MockMaterialization(project, adapter_type="bar")],
+ adapter_type="foo",
+ expected=None,
+ )
+ for project in ["root", "dep", "dbt"]
+ )
+
+ # matching type only, each project
+ sets.extend(
+ FindMaterializationSpec(
+ macros=[MockMaterialization(project, adapter_type="foo")],
+ adapter_type="foo",
+ expected=(project, "foo"),
+ )
+ for project in ["root", "dep", "dbt"]
+ )
+
+ sets.extend(
+ [
+ # matching type and default everywhere
+ FindMaterializationSpec(
+ macros=[
+ MockMaterialization(project, adapter_type=atype)
+ for (project, atype) in product(["root", "dep", "dbt"], ["foo", None])
+ ],
+ adapter_type="foo",
+ expected=("root", "foo"),
+ ),
+ # default in core, override is in dep, and root has unrelated override
+ # should find the dbt default because default materializations cannot be overwritten by packages.
+ FindMaterializationSpec(
+ macros=[
+ MockMaterialization("root", adapter_type="bar"),
+ MockMaterialization("dep", adapter_type="foo"),
+ MockMaterialization("dbt", adapter_type=None),
+ ],
+ adapter_type="foo",
+ expected=("dbt", "default"),
+ ),
+ # default in core, unrelated override is in dep, and root has an override
+ # should find the root override.
+ FindMaterializationSpec(
+ macros=[
+ MockMaterialization("root", adapter_type="foo"),
+ MockMaterialization("dep", adapter_type="bar"),
+ MockMaterialization("dbt", adapter_type=None),
+ ],
+ adapter_type="foo",
+ expected=("root", "foo"),
+ ),
+ # default in core, override is in dep, and root has an override too.
+ # should find the root override.
+ FindMaterializationSpec(
+ macros=[
+ MockMaterialization("root", adapter_type="foo"),
+ MockMaterialization("dep", adapter_type="foo"),
+ MockMaterialization("dbt", adapter_type=None),
+ ],
+ adapter_type="foo",
+ expected=("root", "foo"),
+ ),
+ # core has default + adapter, dep has adapter, root has default
+ # should find the default adapter implementation, because it's the most specific
+ # and default materializations cannot be overwritten by packages
+ FindMaterializationSpec(
+ macros=[
+ MockMaterialization("root", adapter_type=None),
+ MockMaterialization("dep", adapter_type="foo"),
+ MockMaterialization("dbt", adapter_type=None),
+ MockMaterialization("dbt", adapter_type="foo"),
+ ],
+ adapter_type="foo",
+ expected=("dbt", "foo"),
+ ),
+ ]
+ )
+
+ # inherit from parent adapter
+ sets.extend(
+ FindMaterializationSpec(
+ macros=[MockMaterialization(project, adapter_type="foo")],
+ adapter_type="bar",
+ expected=(project, "foo"),
+ )
+ for project in ["root", "dep", "dbt"]
+ )
+ sets.extend(
+ FindMaterializationSpec(
+ macros=[
+ MockMaterialization(project, adapter_type="foo"),
+ MockMaterialization(project, adapter_type="bar"),
+ ],
+ adapter_type="bar",
+ expected=(project, "bar"),
+ )
+ for project in ["root", "dep", "dbt"]
+ )
+
+ return sets
+
+
 @pytest.mark.parametrize(
  "macros,adapter_type,expected",
  _materialization_parameter_sets(),
  ids=id_mat,
 )
 def test_find_materialization_by_name(macros, adapter_type, expected):
+ set_from_args(
+ Namespace(
+ SEND_ANONYMOUS_USAGE_STATS=False,
+ REQUIRE_EXPLICIT_PACKAGE_OVERRIDES_FOR_BUILTIN_MATERIALIZATIONS=True,
+ ),
+ None,
+ )
+
  manifest = make_manifest(macros=macros)
  result = manifest.find_materialization_macro_by_name(
  project_name="root",