Skip to content

Commit

Permalink
add compilation error for invalid setting of store_failures_as
Browse files Browse the repository at this point in the history
  • Loading branch information
mikealfare committed Oct 9, 2023
1 parent e70dca3 commit 8b8f9bb
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 17 deletions.
39 changes: 22 additions & 17 deletions core/dbt/contracts/graph/model_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -575,34 +575,39 @@ def __post_init__(self):
is to include "ephemeral" as a value for `store_failures_as`, which effectively sets
`store_failures=False`.
The exception handling for this is tricky. If we raise an exception here, the entire run fails at
parse time. We would rather well-formed models run successfully, leaving only exceptions to be rerun
if necessary. Hence, the exception needs to be raised in the test materialization. In order to do so,
we need to make sure that we go down the `store_failures = True` route with the invalid setting for
`store_failures_as`. This results in the `.get()` defaulted to `True` below, instead of a normal
dictionary lookup as is done in the `if` block. Refer to the test materialization for the
exception that is raise as a result of an invalid value.
The intention of this block is to behave as if `store_failures_as` is the only setting,
but still allow for backwards compatibility for `store_failures`.
See https://github.com/dbt-labs/dbt-core/issues/6914 for more information.
"""

# if `store_failures_as` is set, it dictates what `store_failures` gets set to
store_failures_map = {
"ephemeral": False,
"table": True,
"view": True,
}

# if `store_failures_as` is not set, it gets set by `store_failures`
store_failures_as_map = {
# the settings below mimic existing behavior prior to `store_failures_as`
get_store_failures_as_map = {
True: "table",
False: "ephemeral",
None: None,
}

if self.store_failures_as in store_failures_map:
self.store_failures = store_failures_map[self.store_failures_as]
elif self.store_failures_as is None:
self.store_failures_as = store_failures_as_map[self.store_failures]
else: # `store_failures_as` is set to an unsupported value
raise CompilationError(
f"""{self.store_failures_as} is not a valid value for `store_failures_as`. """
f"""Accepted values are: {str(list(store_failures_map.keys()))}"""
)
# if `store_failures_as` is set, it dictates what `store_failures` gets set to
# the settings below overrides whatever `store_failures` is set to by the user
get_store_failures_map = {
"ephemeral": False,
"table": True,
"view": True,
}

if self.store_failures_as is None:
self.store_failures_as = get_store_failures_as_map[self.store_failures]
else:
self.store_failures = get_store_failures_map.get(self.store_failures_as, True)

@classmethod
def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> bool:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@
-- if `--store-failures` is invoked via command line and `store_failures_as` is not set,
-- config.get('store_failures_as', 'table') returns None, not 'table'
{% if store_failures_as == none %}{% set store_failures_as = 'table' %}{% endif %}
{% if store_failures_as not in ['table', 'view'] %}
{{ exceptions.raise_compiler_error(
"'" ~ store_failures_as ~ "' is not a valid value for `store_failures_as`. "
"Accepted values are: ['ephemeral', 'table', 'view']"
) }}
{% endif %}

{% set target_relation = api.Relation.create(
identifier=identifier, schema=schema, database=database, type=store_failures_as) -%} %}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,14 @@
"""


TEST__ERROR_UNSET = """
{{ config(store_failures_as="error") }}
select *
from {{ ref('chipmunks') }}
where shirt = 'green'
"""


SCHEMA_YML = """
version: 2
Expand Down
23 changes: 23 additions & 0 deletions tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,3 +280,26 @@ def test_tests_run_successfully_and_are_stored_as_expected(self, project):
TestResult("not_null_chipmunks_shirt", TestStatus.Fail, "view"),
}
self.run_and_assert(project, expected_results)


class StoreTestFailuresAsExceptions(StoreTestFailuresAsBase):
"""
This tests that `store_failures_as` raises exceptions in appropriate scenarios.
Test Scenarios:
- If `store_failures_as = "error"`, a helpful exception is raised.
"""

@pytest.fixture(scope="class")
def tests(self):
return {
"store_failures_as_error.sql": _files.TEST__ERROR_UNSET,
}

def test_tests_run_unsuccessfully_and_raise_appropriate_exception(self, project):
results = run_dbt(["test"], expect_pass=False)
assert len(results) == 1
result = results[0]
assert "Compilation Error" in result.message
assert "'error' is not a valid value" in result.message
assert "Accepted values are: ['ephemeral', 'table', 'view']" in result.message
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
StoreTestFailuresAsProjectLevelView,
StoreTestFailuresAsProjectLevelEphemeral,
StoreTestFailuresAsGeneric,
StoreTestFailuresAsExceptions,
)


Expand Down Expand Up @@ -39,3 +40,7 @@ class TestStoreTestFailuresAsProjectLevelEphemeral(

class TestStoreTestFailuresAsGeneric(StoreTestFailuresAsGeneric, PostgresMixin):
pass


class TestStoreTestFailuresAsExceptions(StoreTestFailuresAsExceptions, PostgresMixin):
pass

0 comments on commit 8b8f9bb

Please sign in to comment.