Skip to content

Commit

Permalink
CT-625: Fail with clear message for invalid materialized vals (#6025)
Browse files Browse the repository at this point in the history
* CT-625: Fail with clear message for invalid materialized vals

* CT-625: Increase test coverage, run pre-commit checks

* CT-625: run black on problem file

* CT-625: Add changelog entry

* CT-625: Remove test that didn't make sense
  • Loading branch information
peterallenwebb authored Oct 14, 2022
1 parent 9b84b6e commit 73aebd8
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 3 deletions.
7 changes: 7 additions & 0 deletions .changes/unreleased/Under the Hood-20221013-181912.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: Under the Hood
body: Provide useful errors when the value of 'materialized' is invalid
time: 2022-10-13T18:19:12.167548-04:00
custom:
Author: peterallenwebb
Issue: "5229"
PR: "6025"
16 changes: 15 additions & 1 deletion core/dbt/contracts/graph/model_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,12 @@ class SeedConfig(NodeConfig):
materialized: str = "seed"
quote_columns: Optional[bool] = None

@classmethod
def validate(cls, data):
super().validate(data)
if data.get("materialized") and data.get("materialized") != "seed":
raise ValidationError("A seed must have a materialized value of 'seed'")


@dataclass
class TestConfig(NodeAndTestConfig):
Expand Down Expand Up @@ -534,6 +540,12 @@ def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> boo
return False
return True

@classmethod
def validate(cls, data):
super().validate(data)
if data.get("materialized") and data.get("materialized") != "test":
raise ValidationError("A test must have a materialized value of 'test'")


@dataclass
class EmptySnapshotConfig(NodeConfig):
Expand Down Expand Up @@ -570,7 +582,6 @@ def validate(cls, data):
f"Invalid value for 'check_cols': {data['check_cols']}. "
"Expected 'all' or a list of strings."
)

elif data.get("strategy") == "timestamp":
if not data.get("updated_at"):
raise ValidationError(
Expand All @@ -582,6 +593,9 @@ def validate(cls, data):
# If the strategy is not 'check' or 'timestamp' it's a custom strategy,
# formerly supported with GenericSnapshotConfig

if data.get("materialized") and data.get("materialized") != "snapshot":
raise ValidationError("A snapshot must have a materialized value of 'snapshot'")

def finalize_and_validate(self):
data = self.to_dict(omit_none=True)
self.validate(data)
Expand Down
15 changes: 15 additions & 0 deletions tests/functional/configs/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,21 @@
- name: my_model_3
"""

simple_snapshot = """{% snapshot mysnapshot %}
{{
config(
target_schema='snapshots',
strategy='timestamp',
unique_key='id',
updated_at='updated_at'
)
}}
select * from dummy
{% endsnapshot %}"""


class BaseConfigProject:
@pytest.fixture(scope="class")
Expand Down
62 changes: 60 additions & 2 deletions tests/functional/configs/test_configs.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@

from hologram import ValidationError
import pytest
import os

from dbt.tests.util import run_dbt, check_relations_equal
from tests.functional.configs.fixtures import BaseConfigProject
from dbt.exceptions import ParsingException
from dbt.tests.util import run_dbt, update_config_file, write_file, check_relations_equal
from tests.functional.configs.fixtures import BaseConfigProject, simple_snapshot


class TestConfigs(BaseConfigProject):
Expand Down Expand Up @@ -63,3 +66,58 @@ def test_alternative_target_paths(self, project):
if os.path.isdir(d) and d.startswith("target_"):
target_path = d
assert os.path.exists(os.path.join(project.project_root, target_path, "manifest.json"))


class TestInvalidTestsMaterializationProj(object):
def test_tests_materialization_proj_config(self, project):
config_patch = {"tests": {"materialized": "table"}}
update_config_file(config_patch, project.project_root, "dbt_project.yml")
tests_dir = os.path.join(project.project_root, "tests")
write_file("select * from foo", tests_dir, "test.sql")

with pytest.raises(ValidationError):
run_dbt()


class TestInvalidSeedsMaterializationProj(object):
def test_seeds_materialization_proj_config(self, project):
config_patch = {"seeds": {"materialized": "table"}}
update_config_file(config_patch, project.project_root, "dbt_project.yml")

seeds_dir = os.path.join(project.project_root, "seeds")
write_file("id1, id2\n1, 2", seeds_dir, "seed.csv")

with pytest.raises(ValidationError):
run_dbt()


class TestInvalidSeedsMaterializationSchema(object):
def test_seeds_materialization_schema_config(self, project):
seeds_dir = os.path.join(project.project_root, "seeds")
write_file("version: 2\nseeds:\n - name: myseed\n config:\n materialized: table", seeds_dir, "schema.yml")
write_file("id1, id2\n1, 2", seeds_dir, "myseed.csv")

with pytest.raises(ValidationError):
run_dbt()


class TestInvalidSnapshotsMaterializationProj(object):
def test_snapshots_materialization_proj_config(self, project):
config_patch = {"snapshots": {"materialized": "table"}}
update_config_file(config_patch, project.project_root, "dbt_project.yml")

snapshots_dir = os.path.join(project.project_root, "snapshots")
write_file(simple_snapshot, snapshots_dir, "mysnapshot.sql")

with pytest.raises(ParsingException):
run_dbt()


class TestInvalidSnapshotsMaterializationSchema(object):
def test_snapshots_materialization_schema_config(self, project):
snapshots_dir = os.path.join(project.project_root, "snapshots")
write_file("version: 2\nsnapshots:\n - name: mysnapshot\n config:\n materialized: table", snapshots_dir, "schema.yml")
write_file(simple_snapshot, snapshots_dir, "mysnapshot.sql")

with pytest.raises(ValidationError):
run_dbt()

0 comments on commit 73aebd8

Please sign in to comment.