Fix: prefer disabled project nodes to external node
resolves https://github.com/dbt-labs/dbt-core/issues/10224
Problem
Currently, an external node can clobber an existing project node if that project node is disabled. This should not be the case as it leads to duplicative nodes in the manifest! (one in manifest.nodes, one in manifest.disabled)
Solution
When injecting an external node to the manifest that already exists from the FS, handle merges by:
- Enabled project nodes are preferred to external nodes
- Disabled project nodes are preferred to external nodes
Checklist
- [x] I have read the contributing guide and understand what's expected of me
- [x] I have run this code in development and it appears to resolve the stated issue
- [x] This PR includes tests, or tests are not required/relevant for this PR
- [x] This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX
- [x] This PR includes type annotations for new and modified functions
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 88.67%. Comparing base (
84456f5) to head (403a1d0).
Additional details and impacted files
@@ Coverage Diff @@
## main #10223 +/- ##
==========================================
- Coverage 88.69% 88.67% -0.03%
==========================================
Files 180 180
Lines 22449 22449
==========================================
- Hits 19912 19906 -6
- Misses 2537 2543 +6
| Flag | Coverage Δ | |
|---|---|---|
| integration | 85.94% <100.00%> (-0.07%) |
:arrow_down: |
| unit | 63.37% <0.00%> (ø) |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.