Skip to content

Commit

Permalink
prefer disabled project nodes to external node
Browse files Browse the repository at this point in the history
  • Loading branch information
MichelleArk committed May 24, 2024
1 parent 84456f5 commit 3d98e50
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 4 deletions.
8 changes: 6 additions & 2 deletions core/dbt/parser/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (

Check warning on line 813 in core/dbt/parser/manifest.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/manifest.py#L813

Added line #L813 was not covered by tests
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

Expand Down
8 changes: 8 additions & 0 deletions tests/functional/partial_parsing/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,14 @@
"""

model_two_disabled_sql = """
{{ config(
enabled=false
) }}
select 1 as notfun
"""

generic_test_schema_yml = """
models:
Expand Down
33 changes: 31 additions & 2 deletions tests/functional/partial_parsing/test_partial_parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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}
Expand All @@ -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()
Expand All @@ -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
Expand Down

0 comments on commit 3d98e50

Please sign in to comment.