From e0783c292231dca50c0f9ca9addb1c2dd45bb7e9 Mon Sep 17 00:00:00 2001 From: Michelle Ark Date: Tue, 28 May 2024 11:37:33 -0400 Subject: [PATCH] Fix: prefer disabled project nodes to external node (#10223) --- .../unreleased/Fixes-20240524-131135.yaml | 6 ++++ core/dbt/parser/manifest.py | 8 +++-- tests/functional/partial_parsing/fixtures.py | 8 +++++ .../partial_parsing/test_partial_parsing.py | 33 +++++++++++++++++-- 4 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 .changes/unreleased/Fixes-20240524-131135.yaml diff --git a/.changes/unreleased/Fixes-20240524-131135.yaml b/.changes/unreleased/Fixes-20240524-131135.yaml new file mode 100644 index 00000000000..7a15d9bf68d --- /dev/null +++ b/.changes/unreleased/Fixes-20240524-131135.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: prefer disabled project nodes to external node +time: 2024-05-24T13:11:35.440443-04:00 +custom: + Author: michelleark + Issue: "10224" diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index 62741b2da6d..49eab1d8551 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -808,8 +808,12 @@ def inject_external_nodes(self) -> bool: plugin_model_nodes = pm.get_nodes().models for node_arg in plugin_model_nodes.values(): node = ModelNode.from_args(node_arg) - # node may already exist from package or running project - in which case we should avoid clobbering it with an external node - if node.unique_id not in self.manifest.nodes: + # node may already exist from package or running project (even if it is disabled), + # in which case we should avoid clobbering it with an external node + if ( + node.unique_id not in self.manifest.nodes + and node.unique_id not in self.manifest.disabled + ): self.manifest.add_node_nofile(node) manifest_nodes_modified = True diff --git a/tests/functional/partial_parsing/fixtures.py b/tests/functional/partial_parsing/fixtures.py index c7a53982ec3..e419a3dfdbd 100644 --- a/tests/functional/partial_parsing/fixtures.py +++ b/tests/functional/partial_parsing/fixtures.py @@ -760,6 +760,14 @@ """ +model_two_disabled_sql = """ +{{ config( + enabled=false +) }} + +select 1 as notfun +""" + generic_test_schema_yml = """ models: diff --git a/tests/functional/partial_parsing/test_partial_parsing.py b/tests/functional/partial_parsing/test_partial_parsing.py index 5a1f496c511..13bb3d5daae 100644 --- a/tests/functional/partial_parsing/test_partial_parsing.py +++ b/tests/functional/partial_parsing/test_partial_parsing.py @@ -50,6 +50,7 @@ model_three_disabled_sql, model_three_modified_sql, model_three_sql, + model_two_disabled_sql, model_two_sql, models_schema1_yml, models_schema2_yml, @@ -695,6 +696,15 @@ def external_model_node_depends_on_parent(self): schema="test_schema", ) + @pytest.fixture(scope="class") + def external_model_node_merge(self): + return ModelNodeArgs( + name="model_two", + package_name="test", + identifier="test_identifier", + schema="test_schema", + ) + @pytest.fixture(scope="class") def models(self): return {"model_one.sql": model_one_sql} @@ -708,6 +718,7 @@ def test_pp_external_models( external_model_node_versioned, external_model_node_depends_on, external_model_node_depends_on_parent, + external_model_node_merge, ): # initial plugin - one external model external_nodes = PluginNodes() @@ -724,12 +735,30 @@ def test_pp_external_models( assert len(manifest.external_node_unique_ids) == 1 assert manifest.external_node_unique_ids == ["model.external.external_model"] - # add a model file + # add a model file - test.model_two write_file(model_two_sql, project.project_root, "models", "model_two.sql") manifest = run_dbt(["--partial-parse", "parse"]) assert len(manifest.nodes) == 3 - # add an external model + # add an external model that is already in project - test.model_two + # project model should be preferred to external model + external_nodes.add_model(external_model_node_merge) + manifest = run_dbt(["--partial-parse", "parse"]) + assert len(manifest.nodes) == 3 + assert len(manifest.external_node_unique_ids) == 1 + + # disable test.model_two in project + # project models should still be preferred to external model + write_file(model_two_disabled_sql, project.project_root, "models", "model_two.sql") + manifest = run_dbt(["--partial-parse", "parse"]) + assert len(manifest.nodes) == 2 + assert len(manifest.disabled) == 1 + assert len(manifest.external_node_unique_ids) == 1 + + # re-enable model_2.sql + write_file(model_two_sql, project.project_root, "models", "model_two.sql") + + # add a new external model external_nodes.add_model(external_model_node_versioned) manifest = run_dbt(["--partial-parse", "parse"]) assert len(manifest.nodes) == 4