dbt-core icon indicating copy to clipboard operation
dbt-core copied to clipboard

Fix: prefer disabled project nodes to external node

Open MichelleArk opened this issue 1 year ago • 2 comments

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

MichelleArk avatar May 24 '24 17:05 MichelleArk

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.

github-actions[bot] avatar May 24 '24 17:05 github-actions[bot]

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.

codecov[bot] avatar May 24 '24 17:05 codecov[bot]