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 changes to model access, deprecation_date, and latest_version in state:modified #8264

Merged
merged 7 commits into from
Aug 10, 2023
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-20230806-222319.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Detect changes to model access, version, or latest_version in state:modified
time: 2023-08-06T22:23:19.166334-04:00
custom:
Author: michelleark
Issue: "8189"
12 changes: 12 additions & 0 deletions core/dbt/contracts/graph/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,18 @@ def search_name(self):
def materialization_enforces_constraints(self) -> bool:
return self.config.materialized in ["table", "incremental"]

def same_contents(self, old, adapter_type) -> bool:
return super().same_contents(old, adapter_type) and self.same_ref_representation(old)

def same_ref_representation(self, old) -> bool:
return (
# Changing the latest_version may break downstream unpinned refs
self.latest_version == old.latest_version
# Changes to access or deprecation_date may lead to ref-related parsing errors
and self.access == old.access
and self.deprecation_date == old.deprecation_date
)

def build_contract_checksum(self):
# We don't need to construct the checksum if the model does not
# have contract enforced, because it won't be used.
Expand Down
2 changes: 1 addition & 1 deletion core/dbt/node_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@


class AccessType(StrEnum):
Protected = "protected"
Private = "private"
Protected = "protected"
Public = "public"

@classmethod
Expand Down
105 changes: 105 additions & 0 deletions tests/functional/defer_state/test_modified_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -781,3 +781,108 @@ def test_modified_body_and_contract(self, project):
# The model's contract has changed, even if non-breaking, so it should be selected by 'state:modified.contract'
results = run_dbt(["list", "-s", "state:modified.contract", "--state", "./state"])
assert results == ["test.my_model"]


modified_table_model_access_yml = """
version: 2
models:
- name: table_model
access: public
"""


class TestModifiedAccess(BaseModifiedState):
def test_changed_access(self, project):
self.run_and_save_state()

# No access change
assert not run_dbt(["list", "-s", "state:modified", "--state", "./state"])

# Modify access (protected -> public)
write_file(modified_table_model_access_yml, "models", "schema.yml")
assert run_dbt(["list", "-s", "state:modified", "--state", "./state"])

results = run_dbt(["list", "-s", "state:modified", "--state", "./state"])
assert results == ["test.table_model"]


modified_table_model_access_yml = """
version: 2
models:
- name: table_model
deprecation_date: 2020-01-01
"""


class TestModifiedDeprecationDate(BaseModifiedState):
def test_changed_access(self, project):
self.run_and_save_state()

# No access change
assert not run_dbt(["list", "-s", "state:modified", "--state", "./state"])

# Modify deprecation_date (None -> 2020-01-01)
write_file(modified_table_model_access_yml, "models", "schema.yml")
assert run_dbt(["list", "-s", "state:modified", "--state", "./state"])

results = run_dbt(["list", "-s", "state:modified", "--state", "./state"])
assert results == ["test.table_model"]


modified_table_model_version_yml = """
version: 2
models:
- name: table_model
versions:
- v: 1
defined_in: table_model
"""


class TestModifiedVersion(BaseModifiedState):
def test_changed_access(self, project):
self.run_and_save_state()

# Change version (null -> v1)
write_file(modified_table_model_version_yml, "models", "schema.yml")

results = run_dbt(["list", "-s", "state:modified", "--state", "./state"])
assert results == ["test.table_model.v1"]


table_model_latest_version_yml = """
version: 2
models:
- name: table_model
latest_version: 1
versions:
- v: 1
defined_in: table_model
"""


modified_table_model_latest_version_yml = """
version: 2
models:
- name: table_model
latest_version: 2
versions:
- v: 1
defined_in: table_model
- v: 2
"""


class TestModifiedLatestVersion(BaseModifiedState):
def test_changed_access(self, project):
# Setup initial latest_version: 1
write_file(table_model_latest_version_yml, "models", "schema.yml")

self.run_and_save_state()

# Bump latest version
write_file(table_model_sql, "models", "table_model_v2.sql")
write_file(modified_table_model_latest_version_yml, "models", "schema.yml")

results = run_dbt(["list", "-s", "state:modified", "--state", "./state"])
assert results == ["test.table_model.v1", "test.table_model.v2"]
6 changes: 6 additions & 0 deletions tests/unit/test_contracts_graph_parsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,8 @@ def test_invalid_bad_materialized(base_parsed_model_dict):
lambda u: (u, u.replace(alias="other")),
lambda u: (u, u.replace(schema="other")),
lambda u: (u, u.replace(database="other")),
# unchanged ref representations - protected is default
lambda u: (u, u.replace(access=AccessType.Protected)),
]


Expand Down Expand Up @@ -485,6 +487,10 @@ def test_invalid_bad_materialized(base_parsed_model_dict):
lambda u: (u, replace_config(u, alias="other")),
lambda u: (u, replace_config(u, schema="other")),
lambda u: (u, replace_config(u, database="other")),
# changed ref representations
lambda u: (u, replace_config(u, access=AccessType.Public)),
lambda u: (u, replace_config(u, latest_version=2)),
lambda u: (u, replace_config(u, version=2)),
]


Expand Down
3 changes: 2 additions & 1 deletion tests/unit/test_graph_selector_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
TestConfig,
TestMetadata,
ColumnInfo,
AccessType,
)
from dbt.contracts.graph.manifest import Manifest, ManifestMetadata
from dbt.contracts.graph.unparsed import ExposureType, Owner
Expand Down Expand Up @@ -125,7 +126,7 @@ def make_model(
checksum=FileHash.from_contents(""),
version=version,
latest_version=latest_version,
access=access,
access=access or AccessType.Protected,
)


Expand Down