Safely remove external nodes from manifest (#8495)

(cherry picked from commit 48d04e8)

.changes/unreleased/Fixes-20230828-125858.yaml

@@ -0,0 +1,7 @@
+kind: Fixes
+body: 'Fix "Internal Error: Expected node <unique-id> not found in manifest" when
+ depends_on set on ModelNodeArgs'
+time: 2023-08-28T12:58:58.061228-04:00
+custom:
+ Author: michelleark
+ Issue: "8506"

core/dbt/parser/manifest.py

@@ -557,7 +557,7 @@ def load(self):
  )
  # parent and child maps will be rebuilt by write_manifest
 
- if not skip_parsing:
+ if not skip_parsing or external_nodes_modified:
  # write out the fully parsed manifest
  self.write_manifest_for_partial_parse()
 
@@ -755,10 +755,13 @@ def write_manifest_for_partial_parse(self):
  def inject_external_nodes(self) -> bool:
  # Remove previously existing external nodes since we are regenerating them
  manifest_nodes_modified = False
+ # Remove all dependent nodes before removing referencing nodes
  for unique_id in self.manifest.external_node_unique_ids:
- self.manifest.nodes.pop(unique_id)
  remove_dependent_project_references(self.manifest, unique_id)
  manifest_nodes_modified = True
+ for unique_id in self.manifest.external_node_unique_ids:
+ # remove external nodes from manifest only after dependent project references safely removed
+ self.manifest.nodes.pop(unique_id)
 
  # Inject any newly-available external nodes
  pm = plugins.get_plugin_manager(self.root_project.project_name)

tests/functional/partial_parsing/test_partial_parsing.py

@@ -760,20 +760,45 @@ def external_model_node_versioned(self):
  version=1,
  )
 
+ @pytest.fixture(scope="class")
+ def external_model_node_depends_on(self):
+ return ModelNodeArgs(
+ name="external_model_depends_on",
+ package_name="external",
+ identifier="test_identifier_depends_on",
+ schema="test_schema",
+ depends_on_nodes=["model.external.external_model_depends_on_parent"],
+ )
+
+ @pytest.fixture(scope="class")
+ def external_model_node_depends_on_parent(self):
+ return ModelNodeArgs(
+ name="external_model_depends_on_parent",
+ package_name="external",
+ identifier="test_identifier_depends_on_parent",
+ schema="test_schema",
+ )
+
  @pytest.fixture(scope="class")
  def models(self):
  return {"model_one.sql": model_one_sql}
 
  @mock.patch("dbt.plugins.get_plugin_manager")
  def test_pp_external_models(
- self, get_plugin_manager, project, external_model_node, external_model_node_versioned
+ self,
+ get_plugin_manager,
+ project,
+ external_model_node,
+ external_model_node_versioned,
+ external_model_node_depends_on,
+ external_model_node_depends_on_parent,
  ):
  # initial plugin - one external model
  external_nodes = PluginNodes()
  external_nodes.add_model(external_model_node)
  get_plugin_manager.return_value.get_nodes.return_value = external_nodes
 
- # initial run
+ # initial parse
  manifest = run_dbt(["parse"])
  assert len(manifest.nodes) == 2
  assert set(manifest.nodes.keys()) == {
@@ -803,8 +828,21 @@ def test_pp_external_models(
  )
  manifest = run_dbt(["--partial-parse", "parse"])
  assert len(manifest.nodes) == 5
+ assert len(manifest.external_node_unique_ids) == 2
 
  # remove a model file that depends on external model
  rm_file(project.project_root, "models", "model_depends_on_external.sql")
  manifest = run_dbt(["--partial-parse", "parse"])
  assert len(manifest.nodes) == 4
+
+ # add an external node with depends on
+ external_nodes.add_model(external_model_node_depends_on)
+ external_nodes.add_model(external_model_node_depends_on_parent)
+ manifest = run_dbt(["--partial-parse", "parse"])
+ assert len(manifest.nodes) == 6
+ assert len(manifest.external_node_unique_ids) == 4
+
+ # skip files parsing - ensure no issues
+ run_dbt(["--partial-parse", "parse"])
+ assert len(manifest.nodes) == 6
+ assert len(manifest.external_node_unique_ids) == 4