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

Detect breaking changes to column names and data types in state:modified check #7216

Merged
merged 27 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
b66beb6
Add state.modified.contract and tests
gshank Mar 22, 2023
14a8e30
Fix unit test
gshank Mar 23, 2023
c5b73ec
Update manifest/v9.json for artifact tests
gshank Mar 23, 2023
2f0d6b7
Update artifacts tests. Add reason to exception.
gshank Mar 23, 2023
d3a4cf4
first pass at changes before modifying tests
emmyoop Mar 21, 2023
edd9261
test updates
emmyoop Mar 22, 2023
b028400
add default
emmyoop Mar 24, 2023
7abbf67
update manifest
emmyoop Mar 24, 2023
4ee1a12
fix tests
emmyoop Mar 24, 2023
1cbab4c
changelog
emmyoop Mar 24, 2023
95d112c
fix unit tests
emmyoop Mar 24, 2023
8540fd3
rename strict -> enforced
emmyoop Mar 24, 2023
25be193
Move call to build_contract_checksum, concatenate reasons and update
gshank Mar 24, 2023
9311211
Merge branch 'main' into ct-2038-contract_state_modified
gshank Mar 24, 2023
d7ed116
Expand test a bit
gshank Mar 27, 2023
9498809
convert to object
emmyoop Mar 27, 2023
c711451
fix tests
emmyoop Mar 27, 2023
8943468
Update Under the Hood-20230217-105223.yaml
emmyoop Mar 27, 2023
20cd0f3
Update Under the Hood-20230217-105223.yaml
emmyoop Mar 27, 2023
520fc40
Rearrange same_contract
gshank Mar 27, 2023
4bf2766
remove stray breakpoints
emmyoop Mar 27, 2023
449aa48
move Contract definition to model_config
emmyoop Mar 27, 2023
8863089
Merge branch 'er/ct-2314-contract-dict' into ct-2038-contract_state_m…
gshank Mar 28, 2023
382f66f
Make changes to use new Contract object
gshank Mar 28, 2023
4f5a5ae
Merge branch 'main' into ct-2038-contract_state_modified
gshank Mar 28, 2023
4b2213a
Fix reference to self.contract in model_config.py
gshank Mar 28, 2023
46ad6d3
Merge branch 'main' into ct-2038-contract_state_modified
gshank Mar 28, 2023
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/Features-20230323-133026.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: Detect breaking changes to contracts in state:modified check
time: 2023-03-23T13:30:26.593717-04:00
custom:
Author: gshank
Issue: "6869"
45 changes: 44 additions & 1 deletion core/dbt/contracts/graph/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import time
from dataclasses import dataclass, field
from enum import Enum
import hashlib

from mashumaro.types import SerializableType
from typing import (
Expand Down Expand Up @@ -39,7 +40,7 @@
from dbt.contracts.util import Replaceable, AdditionalPropertiesMixin
from dbt.events.proto_types import NodeInfo
from dbt.events.functions import warn_or_error
from dbt.exceptions import ParsingError, InvalidAccessTypeError
from dbt.exceptions import ParsingError, InvalidAccessTypeError, ModelContractError
from dbt.events.types import (
SeedIncreased,
SeedExceedsLimitSamePath,
Expand Down Expand Up @@ -387,6 +388,13 @@ def same_config(self, old) -> bool:
old.unrendered_config,
)

def build_contract_checksum(self):
pass

def same_contract(self, old) -> bool:
# This would only apply to seeds
return True

def patch(self, patch: "ParsedNodePatch"):
"""Given a ParsedNodePatch, add the new information to the node."""
# explicitly pick out the parts to update so we don't inadvertently
Expand Down Expand Up @@ -428,6 +436,7 @@ def same_contents(self, old) -> bool:
and self.same_persisted_description(old)
and self.same_fqn(old)
and self.same_database_representation(old)
and self.same_contract(old)
and True
)

Expand Down Expand Up @@ -457,6 +466,7 @@ class CompiledNode(ParsedNode):
extra_ctes: List[InjectedCTE] = field(default_factory=list)
_pre_injected_sql: Optional[str] = None
contract: bool = False
contract_checksum: Optional[str] = None

@property
def empty(self):
Expand Down Expand Up @@ -497,6 +507,39 @@ def depends_on_nodes(self):
def depends_on_macros(self):
return self.depends_on.macros

def build_contract_checksum(self):
# We don't need to construct the checksum if the model does not
# have contract enabled, because it won't be used.
# This needs to be executed after contract config is set
if self.contract is True:
contract_state = ""
# We need to sort the columns so that order doesn't matter
# columns is a str: ColumnInfo dictionary
sorted_columns = sorted(self.columns.values(), key=lambda col: col.name)
for column in sorted_columns:
contract_state += f"|{column.name}"
contract_state += column.data_type
contract_state += str(column.constraints)
data = contract_state.encode("utf-8")
self.contract_checksum = hashlib.new("sha256", data).hexdigest()

def same_contract(self, old) -> bool:
if old.contract is False and self.contract is False:
# Not a change
return True
if old.contract is False and self.contract is True:
# A change, but not a breaking change
return False
if old.contract is True and self.contract is False:
# Breaking change: throw an error
raise (ModelContractError(reason="contract has been disabled", node=self))
if self.contract_checksum == old.contract_checksum:
MichelleArk marked this conversation as resolved.
Show resolved Hide resolved
# Breaking change: throw an error
raise (ModelContractError(reason="column definitions have changed", node=self))
MichelleArk marked this conversation as resolved.
Show resolved Hide resolved
else:
# No change
return False
MichelleArk marked this conversation as resolved.
Show resolved Hide resolved


# ====================================
# CompiledNode subclasses
Expand Down
19 changes: 19 additions & 0 deletions core/dbt/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,25 @@ def _fix_dupe_msg(self, path_1: str, path_2: str, name: str, type_name: str) ->
)


class ModelContractError(DbtRuntimeError):
CODE = 10016
MESSAGE = "Contract Error"

def __init__(self, reason, node=None):
self.reason = reason
super().__init__(self.message(), node)

@property
def type(self):
return "Contract"

def message(self):
return (
"There is a breaking change in the model contract because {self.reason}; "
MichelleArk marked this conversation as resolved.
Show resolved Hide resolved
"you may need to create a new version. See: https://docs.getdbt.com/docs/collaborate/publish/model-versions"
)


class RecursionError(DbtRuntimeError):
pass

Expand Down
1 change: 1 addition & 0 deletions core/dbt/graph/selector_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,7 @@ def search(self, included_nodes: Set[UniqueId], selector: str) -> Iterator[Uniqu
),
"modified.relation": self.check_modified_factory("same_database_representation"),
"modified.macros": self.check_modified_macros,
"modified.contract": self.check_modified_factory("same_contract"),
}
if selector in state_checks:
checker = state_checks[selector]
Expand Down
1 change: 1 addition & 0 deletions core/dbt/parser/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ def update_parsed_node_config(
# compatibility with earlier node-only config.
if config_dict.get("contract", False):
parsed_node.contract = True
parsed_node.build_contract_checksum()
MichelleArk marked this conversation as resolved.
Show resolved Hide resolved

# unrendered_config is used to compare the original database/schema/alias
# values and to handle 'same_config' and 'same_contents' calls
Expand Down
Loading