Skip to content

Commit

Permalink
Check modified contracts when doing state:modified (#11161)
Browse files Browse the repository at this point in the history
  • Loading branch information
gshank authored Dec 18, 2024
1 parent 6076cf7 commit 88e953e
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 3 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20241217-154848.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Run check_modified_contract for state:modified
time: 2024-12-17T15:48:48.053054-05:00
custom:
Author: gshank
Issue: "11034"
17 changes: 14 additions & 3 deletions core/dbt/graph/selector_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -680,17 +680,24 @@ def check_macros_modified(self, node):
def check_modified_content(
self, old: Optional[SelectorTarget], new: SelectorTarget, adapter_type: str
) -> bool:
different_contents = False
if isinstance(
new,
(SourceDefinition, Exposure, Metric, SemanticModel, UnitTestDefinition, SavedQuery),
):
# these all overwrite `same_contents`
different_contents = not new.same_contents(old) # type: ignore
else:
elif new: # because we also pull in deleted/disabled nodes, this could be None
different_contents = not new.same_contents(old, adapter_type) # type: ignore

upstream_macro_change = self.check_macros_modified(new)
return different_contents or upstream_macro_change

check_modified_contract = False
if isinstance(old, ModelNode):
func = self.check_modified_contract("same_contract", adapter_type)
check_modified_contract = func(old, new)

return different_contents or upstream_macro_change or check_modified_contract

def check_unmodified_content(
self, old: Optional[SelectorTarget], new: SelectorTarget, adapter_type: str
Expand Down Expand Up @@ -792,7 +799,11 @@ def search(self, included_nodes: Set[UniqueId], selector: str) -> Iterator[Uniqu
yield unique_id

# checkers that can handle removed nodes
if checker.__name__ in ["check_modified_contract"]:
if checker.__name__ in [
"check_modified_contract",
"check_modified_content",
"check_unmodified_content",
]:
# ignore included_nodes, since those cannot contain removed nodes
for previous_unique_id, previous_node in manifest.nodes.items():
# detect removed (deleted, renamed, or disabled) nodes
Expand Down
17 changes: 17 additions & 0 deletions tests/functional/defer_state/test_modified_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,15 @@ def test_delete_unversioned_contracted_model(self, project):
assert expected_warning in logs
assert expected_change in logs

# the same but for general-purpose state:modified
_, logs = run_dbt_and_capture(
["run", "--models", "state:modified.contract", "--state", "./state"]
)
expected_warning = "While comparing to previous project state, dbt detected a breaking change to an unversioned model"
expected_change = "Contracted model 'model.test.table_model' was deleted or renamed"
assert expected_warning in logs
assert expected_change in logs


class TestDeleteVersionedContractedModel(BaseModifiedState):
MODEL_UNIQUE_ID = "model.test.table_model.v1"
Expand All @@ -697,6 +706,14 @@ def test_delete_versioned_contracted_model(self, project):
e.value
)

# the same but for general-purpose state:modified
with pytest.raises(ContractBreakingChangeError) as e:
run_dbt(["run", "--models", "state:modified", "--state", "./state"])

assert "Contracted model 'model.test.table_model.v1' was deleted or renamed." in str(
e.value
)


class TestDisableUnversionedContractedModel(BaseModifiedState):
MODEL_UNIQUE_ID = "model.test.table_model"
Expand Down

0 comments on commit 88e953e

Please sign in to comment.