From 94448ddad9c7e06d7c8bd3b2c195c74e24562707 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Tue, 15 Oct 2024 11:50:48 -0400 Subject: [PATCH 1/8] Refactor code to properly handle reference deprecations --- core/dbt/contracts/graph/manifest.py | 19 ---------- core/dbt/parser/manifest.py | 55 ++++++++++++++-------------- 2 files changed, 28 insertions(+), 46 deletions(-) diff --git a/core/dbt/contracts/graph/manifest.py b/core/dbt/contracts/graph/manifest.py index b556b479fb4..4ce887591d9 100644 --- a/core/dbt/contracts/graph/manifest.py +++ b/core/dbt/contracts/graph/manifest.py @@ -1315,25 +1315,6 @@ def singular_test_lookup(self) -> SingularTestLookup: def external_node_unique_ids(self): return [node.unique_id for node in self.nodes.values() if node.is_external_node] - def resolve_refs( - self, - source_node: ModelNode, - current_project: str, # TODO: ModelNode is overly restrictive typing - ) -> List[MaybeNonSource]: - resolved_refs: List[MaybeNonSource] = [] - for ref in source_node.refs: - resolved = self.resolve_ref( - source_node, - ref.name, - ref.package, - ref.version, - current_project, - source_node.package_name, - ) - resolved_refs.append(resolved) - - return resolved_refs - # Called by dbt.parser.manifest._process_refs & ManifestLoader.check_for_model_deprecations def resolve_ref( self, diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index 7ffd00febc5..155a4e72d87 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -574,35 +574,36 @@ def safe_update_project_parser_files_partially(self, project_parser_files: Dict) def check_for_model_deprecations(self): for node in self.manifest.nodes.values(): - if isinstance(node, ModelNode) and node.is_past_deprecation_date: - warn_or_error( - DeprecatedModel( - model_name=node.name, - model_version=version_to_str(node.version), - deprecation_date=node.deprecation_date.isoformat(), + if isinstance(node, ModelNode) and node.deprecation_date: + if node.is_past_deprecation_date: + warn_or_error( + DeprecatedModel( + model_name=node.name, + model_version=version_to_str(node.version), + deprecation_date=node.deprecation_date.isoformat(), + ) ) - ) - - resolved_refs = self.manifest.resolve_refs(node, self.root_project.project_name) - resolved_model_refs = [r for r in resolved_refs if isinstance(r, ModelNode)] - node.depends_on - for resolved_ref in resolved_model_refs: - if resolved_ref.deprecation_date: - if resolved_ref.is_past_deprecation_date: - event_cls = DeprecatedReference - else: - event_cls = UpcomingReferenceDeprecation - - warn_or_error( - event_cls( - model_name=node.name, - ref_model_package=resolved_ref.package_name, - ref_model_name=resolved_ref.name, - ref_model_version=version_to_str(resolved_ref.version), - ref_model_latest_version=str(resolved_ref.latest_version), - ref_model_deprecation_date=resolved_ref.deprecation_date.isoformat(), - ) + # At this point _process_refs should already have been called, and + # the parent and child maps rebuilt. + # Get the child_nodes and check for deprecations. + child_nodes = self.child_map[node.unique_id] + for child_unique_id in child_nodes: + child_node = self.nodes[child_unique_id] + if node.is_past_deprecation_date: + event_cls = DeprecatedReference + else: + event_cls = UpcomingReferenceDeprecation + + warn_or_error( + event_cls( + model_name=child_node.name, + ref_model_package=node.package_name, + ref_model_name=node.name, + ref_model_version=version_to_str(node.version), + ref_model_latest_version=str(node.latest_version), + ref_model_deprecation_date=node.deprecation_date.isoformat(), ) + ) def check_for_spaces_in_resource_names(self): """Validates that resource names do not contain spaces From 990b56308af6795d5d18431e1606711658350898 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Tue, 15 Oct 2024 12:18:33 -0400 Subject: [PATCH 2/8] Changie --- .changes/unreleased/Fixes-20241015-121825.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20241015-121825.yaml diff --git a/.changes/unreleased/Fixes-20241015-121825.yaml b/.changes/unreleased/Fixes-20241015-121825.yaml new file mode 100644 index 00000000000..f5867871868 --- /dev/null +++ b/.changes/unreleased/Fixes-20241015-121825.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Fix warnings for models referring to a deprecated model +time: 2024-10-15T12:18:25.14525-04:00 +custom: + Author: gshank + Issue: "10833" From 1ad42dfa1cd2890d1a2919abc75bbc3f54558934 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Tue, 15 Oct 2024 12:27:55 -0400 Subject: [PATCH 3/8] Rename tests/functional/deprecations --- .../functional/{deprecations => deprecation_warnings}/fixtures.py | 0 .../{deprecations => deprecation_warnings}/model_deprecations.py | 0 .../test_config_deprecations.py | 0 .../{deprecations => deprecation_warnings}/test_deprecations.py | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename tests/functional/{deprecations => deprecation_warnings}/fixtures.py (100%) rename tests/functional/{deprecations => deprecation_warnings}/model_deprecations.py (100%) rename tests/functional/{deprecations => deprecation_warnings}/test_config_deprecations.py (100%) rename tests/functional/{deprecations => deprecation_warnings}/test_deprecations.py (100%) diff --git a/tests/functional/deprecations/fixtures.py b/tests/functional/deprecation_warnings/fixtures.py similarity index 100% rename from tests/functional/deprecations/fixtures.py rename to tests/functional/deprecation_warnings/fixtures.py diff --git a/tests/functional/deprecations/model_deprecations.py b/tests/functional/deprecation_warnings/model_deprecations.py similarity index 100% rename from tests/functional/deprecations/model_deprecations.py rename to tests/functional/deprecation_warnings/model_deprecations.py diff --git a/tests/functional/deprecations/test_config_deprecations.py b/tests/functional/deprecation_warnings/test_config_deprecations.py similarity index 100% rename from tests/functional/deprecations/test_config_deprecations.py rename to tests/functional/deprecation_warnings/test_config_deprecations.py diff --git a/tests/functional/deprecations/test_deprecations.py b/tests/functional/deprecation_warnings/test_deprecations.py similarity index 100% rename from tests/functional/deprecations/test_deprecations.py rename to tests/functional/deprecation_warnings/test_deprecations.py From 4bc3533043c7d2140dcac745710d821944e80cec Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Tue, 15 Oct 2024 12:32:23 -0400 Subject: [PATCH 4/8] Fix up deprecation tests --- tests/functional/deprecation_warnings/model_deprecations.py | 2 +- .../functional/deprecation_warnings/test_config_deprecations.py | 2 +- tests/functional/deprecation_warnings/test_deprecations.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/functional/deprecation_warnings/model_deprecations.py b/tests/functional/deprecation_warnings/model_deprecations.py index 03e38b1220e..24e7ec47f44 100644 --- a/tests/functional/deprecation_warnings/model_deprecations.py +++ b/tests/functional/deprecation_warnings/model_deprecations.py @@ -1,8 +1,8 @@ import pytest from dbt.cli.main import dbtRunner -from dbt.exceptions import EventCompilationError from dbt.tests.util import run_dbt +from dbt_common.exceptions import EventCompilationError deprecated_model__yml = """ version: 2 diff --git a/tests/functional/deprecation_warnings/test_config_deprecations.py b/tests/functional/deprecation_warnings/test_config_deprecations.py index 077dd7da103..268671cf5fe 100644 --- a/tests/functional/deprecation_warnings/test_config_deprecations.py +++ b/tests/functional/deprecation_warnings/test_config_deprecations.py @@ -4,7 +4,7 @@ from dbt.exceptions import CompilationError, ProjectContractError, YamlParseDictError from dbt.tests.fixtures.project import write_project_files from dbt.tests.util import run_dbt, update_config_file -from tests.functional.deprecations.fixtures import ( +from tests.functional.deprecation_warnings.fixtures import ( data_tests_yaml, local_dependency__dbt_project_yml, local_dependency__schema_yml, diff --git a/tests/functional/deprecation_warnings/test_deprecations.py b/tests/functional/deprecation_warnings/test_deprecations.py index 95779338d8a..0e156f00ddd 100644 --- a/tests/functional/deprecation_warnings/test_deprecations.py +++ b/tests/functional/deprecation_warnings/test_deprecations.py @@ -5,7 +5,7 @@ from dbt import deprecations 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 ( +from tests.functional.deprecation_warnings.fixtures import ( bad_name_yaml, models_trivial__model_sql, ) From f3ecacb8b3c8dce7cde83604b954d68b40a132e4 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Tue, 15 Oct 2024 13:19:28 -0400 Subject: [PATCH 5/8] A couple of typos --- core/dbt/parser/manifest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index 155a4e72d87..aaa7c6a86be 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -586,9 +586,9 @@ def check_for_model_deprecations(self): # At this point _process_refs should already have been called, and # the parent and child maps rebuilt. # Get the child_nodes and check for deprecations. - child_nodes = self.child_map[node.unique_id] + child_nodes = self.manifest.child_map[node.unique_id] for child_unique_id in child_nodes: - child_node = self.nodes[child_unique_id] + child_node = self.manifest.nodes[child_unique_id] if node.is_past_deprecation_date: event_cls = DeprecatedReference else: From c5d09d18d9c9468bbd7ffa26333214d680dc184e Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Tue, 15 Oct 2024 13:25:10 -0400 Subject: [PATCH 6/8] remove rename of dir, rename test --- .../{deprecation_warnings => deprecations}/fixtures.py | 0 .../test_config_deprecations.py | 2 +- .../{deprecation_warnings => deprecations}/test_deprecations.py | 2 +- .../test_model_deprecations.py} | 0 4 files changed, 2 insertions(+), 2 deletions(-) rename tests/functional/{deprecation_warnings => deprecations}/fixtures.py (100%) rename tests/functional/{deprecation_warnings => deprecations}/test_config_deprecations.py (98%) rename tests/functional/{deprecation_warnings => deprecations}/test_deprecations.py (99%) rename tests/functional/{deprecation_warnings/model_deprecations.py => deprecations/test_model_deprecations.py} (100%) diff --git a/tests/functional/deprecation_warnings/fixtures.py b/tests/functional/deprecations/fixtures.py similarity index 100% rename from tests/functional/deprecation_warnings/fixtures.py rename to tests/functional/deprecations/fixtures.py diff --git a/tests/functional/deprecation_warnings/test_config_deprecations.py b/tests/functional/deprecations/test_config_deprecations.py similarity index 98% rename from tests/functional/deprecation_warnings/test_config_deprecations.py rename to tests/functional/deprecations/test_config_deprecations.py index 268671cf5fe..077dd7da103 100644 --- a/tests/functional/deprecation_warnings/test_config_deprecations.py +++ b/tests/functional/deprecations/test_config_deprecations.py @@ -4,7 +4,7 @@ from dbt.exceptions import CompilationError, ProjectContractError, YamlParseDictError from dbt.tests.fixtures.project import write_project_files from dbt.tests.util import run_dbt, update_config_file -from tests.functional.deprecation_warnings.fixtures import ( +from tests.functional.deprecations.fixtures import ( data_tests_yaml, local_dependency__dbt_project_yml, local_dependency__schema_yml, diff --git a/tests/functional/deprecation_warnings/test_deprecations.py b/tests/functional/deprecations/test_deprecations.py similarity index 99% rename from tests/functional/deprecation_warnings/test_deprecations.py rename to tests/functional/deprecations/test_deprecations.py index 0e156f00ddd..95779338d8a 100644 --- a/tests/functional/deprecation_warnings/test_deprecations.py +++ b/tests/functional/deprecations/test_deprecations.py @@ -5,7 +5,7 @@ from dbt import deprecations from dbt.tests.util import run_dbt, run_dbt_and_capture, write_file from dbt_common.exceptions import EventCompilationError -from tests.functional.deprecation_warnings.fixtures import ( +from tests.functional.deprecations.fixtures import ( bad_name_yaml, models_trivial__model_sql, ) diff --git a/tests/functional/deprecation_warnings/model_deprecations.py b/tests/functional/deprecations/test_model_deprecations.py similarity index 100% rename from tests/functional/deprecation_warnings/model_deprecations.py rename to tests/functional/deprecations/test_model_deprecations.py From 9f377cba22b433f7ed943a092b11008e492c955b Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Tue, 15 Oct 2024 13:52:19 -0400 Subject: [PATCH 7/8] Explicitly rebuild the parent & child maps --- core/dbt/parser/manifest.py | 4 +++- tests/functional/deprecations/test_model_deprecations.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index aaa7c6a86be..588be482a8f 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -573,6 +573,8 @@ def safe_update_project_parser_files_partially(self, project_parser_files: Dict) return project_parser_files def check_for_model_deprecations(self): + # build parent and child_maps + self.manifest.build_parent_and_child_maps() for node in self.manifest.nodes.values(): if isinstance(node, ModelNode) and node.deprecation_date: if node.is_past_deprecation_date: @@ -584,7 +586,7 @@ def check_for_model_deprecations(self): ) ) # At this point _process_refs should already have been called, and - # the parent and child maps rebuilt. + # we just rebuilt the parent and child maps. # Get the child_nodes and check for deprecations. child_nodes = self.manifest.child_map[node.unique_id] for child_unique_id in child_nodes: diff --git a/tests/functional/deprecations/test_model_deprecations.py b/tests/functional/deprecations/test_model_deprecations.py index 24e7ec47f44..1318562c10f 100644 --- a/tests/functional/deprecations/test_model_deprecations.py +++ b/tests/functional/deprecations/test_model_deprecations.py @@ -52,7 +52,7 @@ def test_deprecation_warning_error_options(self, project): run_dbt(["--warn-error-options", '{"include": ["DeprecatedModel"]}', "parse"]) -class TestUpcomingReferenceDeprecatingWarning: +class TestUpcomingReferenceDeprecationWarning: @pytest.fixture(scope="class") def models(self): return { From ccea0673c0f6cb23895464d4cd7cf14856ada179 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Tue, 15 Oct 2024 16:14:40 -0400 Subject: [PATCH 8/8] Check type of child_node --- core/dbt/parser/manifest.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index 588be482a8f..493b562bbdc 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -591,6 +591,8 @@ def check_for_model_deprecations(self): child_nodes = self.manifest.child_map[node.unique_id] for child_unique_id in child_nodes: child_node = self.manifest.nodes[child_unique_id] + if not isinstance(child_node, ModelNode): + continue if node.is_past_deprecation_date: event_cls = DeprecatedReference else: