Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 1.9.latest] Check modified contracts when doing state:modified #11168

Merged
merged 1 commit into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading