Safely remove external nodes from manifest (#8495)

.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

@@ -559,7 +559,7 @@ def load(self) -> Manifest:
                 )
                 # 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()
 
@@ -757,10 +757,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