From e70dca3c33a1be2f5ce212af123b54ad35e9981c Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Mon, 9 Oct 2023 18:39:04 -0400 Subject: [PATCH] add ephemeral option for store_failures_as, as a way to easily turn off store_failures at the model level --- core/dbt/contracts/graph/model_config.py | 34 +++++- core/dbt/contracts/relation.py | 1 + .../macros/materializations/tests/test.sql | 2 +- .../store_test_failures_tests/_files.py | 32 +++-- .../store_test_failures_tests/basic.py | 115 +++++++++++------- .../test_store_test_failures.py | 39 +++--- 6 files changed, 148 insertions(+), 75 deletions(-) diff --git a/core/dbt/contracts/graph/model_config.py b/core/dbt/contracts/graph/model_config.py index a633c97d89a..4873ec83d19 100644 --- a/core/dbt/contracts/graph/model_config.py +++ b/core/dbt/contracts/graph/model_config.py @@ -569,14 +569,40 @@ def __post_init__(self): configure this. Hence, if `store_failures = True` and `store_failures_as` is not specified, then it should be set to "table" to mimic the existing functionality. + A side effect of this overriding functionality is that `store_failures_as="view"` at the project + level cannot be turned off at the model level without setting both `store_failures_as` and + `store_failures`. The former would cascade down and override `store_failures=False`. The proposal + is to include "ephemeral" as a value for `store_failures_as`, which effectively sets + `store_failures=False`. + 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 self.store_failures_as: - self.store_failures = True - elif self.store_failures: - self.store_failures_as = "table" + + # 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 = { + 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()))}""" + ) @classmethod def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> bool: diff --git a/core/dbt/contracts/relation.py b/core/dbt/contracts/relation.py index 2cf811f9f6c..52f7a07976f 100644 --- a/core/dbt/contracts/relation.py +++ b/core/dbt/contracts/relation.py @@ -19,6 +19,7 @@ class RelationType(StrEnum): CTE = "cte" MaterializedView = "materialized_view" External = "external" + Ephemeral = "ephemeral" class ComponentName(StrEnum): diff --git a/core/dbt/include/global_project/macros/materializations/tests/test.sql b/core/dbt/include/global_project/macros/materializations/tests/test.sql index 75fb47421eb..238faf3fd1c 100644 --- a/core/dbt/include/global_project/macros/materializations/tests/test.sql +++ b/core/dbt/include/global_project/macros/materializations/tests/test.sql @@ -7,7 +7,7 @@ {% set identifier = model['alias'] %} {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} - {% set store_failures_as = config.get('store_failures_as', 'table') %} + {% set store_failures_as = config.get('store_failures_as') %} -- 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 %} diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py index 38d73141271..d6302ff56c8 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py @@ -62,6 +62,30 @@ """ +TEST__EPHEMERAL_TRUE = """ +{{ config(store_failures_as="ephemeral", store_failures=True) }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__EPHEMERAL_FALSE = """ +{{ config(store_failures_as="ephemeral", store_failures=False) }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + +TEST__EPHEMERAL_UNSET = """ +{{ config(store_failures_as="ephemeral") }} +select * +from {{ ref('chipmunks') }} +where shirt = 'green' +""" + + TEST__UNSET_TRUE = """ {{ config(store_failures=True) }} select * @@ -93,14 +117,6 @@ """ -TEST__NONE_FALSE = """ -{{ config(store_failures_as=None, store_failures=False) }} -select * -from {{ ref('chipmunks') }} -where shirt = 'green' -""" - - SCHEMA_YML = """ version: 2 diff --git a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py index 4f84beb4960..cfdd59d3c14 100644 --- a/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py +++ b/tests/adapter/dbt/tests/adapter/store_test_failures_tests/basic.py @@ -6,22 +6,7 @@ from dbt.contracts.results import TestStatus from dbt.tests.util import run_dbt, check_relation_types -from dbt.tests.adapter.store_test_failures_tests._files import ( - MODEL__CHIPMUNKS, - SCHEMA_YML, - SEED__CHIPMUNKS, - TEST__NONE_FALSE, - TEST__TABLE_FALSE, - TEST__TABLE_TRUE, - TEST__TABLE_UNSET, - TEST__UNSET_FALSE, - TEST__UNSET_TRUE, - TEST__UNSET_UNSET, - TEST__VIEW_FALSE, - TEST__VIEW_TRUE, - TEST__VIEW_UNSET, - TEST__VIEW_UNSET_PASS, -) +from dbt.tests.adapter.store_test_failures_tests import _files TestResult = namedtuple("TestResult", ["name", "status", "type"]) @@ -62,11 +47,11 @@ def teardown_method(self, project): @pytest.fixture(scope="class") def seeds(self): - return {f"{self.seed_table}.csv": SEED__CHIPMUNKS} + return {f"{self.seed_table}.csv": _files.SEED__CHIPMUNKS} @pytest.fixture(scope="class") def models(self): - return {f"{self.model_table}.sql": MODEL__CHIPMUNKS} + return {f"{self.model_table}.sql": _files.MODEL__CHIPMUNKS} def run_and_assert( self, project, expected_results: Set[TestResult], expect_pass: bool = False @@ -109,6 +94,9 @@ class StoreTestFailuresAsInteractions(StoreTestFailuresAsBase): - If `store_failures_as = "table"` and `store_failures = True`, then store the failures in a table. - If `store_failures_as = "table"` and `store_failures = False`, then store the failures in a table. - If `store_failures_as = "table"` and `store_failures` is not set, then store the failures in a table. + - If `store_failures_as = "ephemeral"` and `store_failures = True`, then do not store the failures. + - If `store_failures_as = "ephemeral"` and `store_failures = False`, then do not store the failures. + - If `store_failures_as = "ephemeral"` and `store_failures` is not set, then do not store the failures. - If `store_failures_as` is not set and `store_failures = True`, then store the failures in a table. - If `store_failures_as` is not set and `store_failures = False`, then do not store the failures. - If `store_failures_as` is not set and `store_failures` is not set, then do not store the failures. @@ -117,16 +105,19 @@ class StoreTestFailuresAsInteractions(StoreTestFailuresAsBase): @pytest.fixture(scope="class") def tests(self): return { - "view_unset_pass.sql": TEST__VIEW_UNSET_PASS, # control - "view_true.sql": TEST__VIEW_TRUE, - "view_false.sql": TEST__VIEW_FALSE, - "view_unset.sql": TEST__VIEW_UNSET, - "table_true.sql": TEST__TABLE_TRUE, - "table_false.sql": TEST__TABLE_FALSE, - "table_unset.sql": TEST__TABLE_UNSET, - "unset_true.sql": TEST__UNSET_TRUE, - "unset_false.sql": TEST__UNSET_FALSE, - "unset_unset.sql": TEST__UNSET_UNSET, + "view_unset_pass.sql": _files.TEST__VIEW_UNSET_PASS, # control + "view_true.sql": _files.TEST__VIEW_TRUE, + "view_false.sql": _files.TEST__VIEW_FALSE, + "view_unset.sql": _files.TEST__VIEW_UNSET, + "table_true.sql": _files.TEST__TABLE_TRUE, + "table_false.sql": _files.TEST__TABLE_FALSE, + "table_unset.sql": _files.TEST__TABLE_UNSET, + "ephemeral_true.sql": _files.TEST__EPHEMERAL_TRUE, + "ephemeral_false.sql": _files.TEST__EPHEMERAL_FALSE, + "ephemeral_unset.sql": _files.TEST__EPHEMERAL_UNSET, + "unset_true.sql": _files.TEST__UNSET_TRUE, + "unset_false.sql": _files.TEST__UNSET_FALSE, + "unset_unset.sql": _files.TEST__UNSET_UNSET, } def test_tests_run_successfully_and_are_stored_as_expected(self, project): @@ -138,6 +129,9 @@ def test_tests_run_successfully_and_are_stored_as_expected(self, project): TestResult("table_true", TestStatus.Fail, "table"), TestResult("table_false", TestStatus.Fail, "table"), TestResult("table_unset", TestStatus.Fail, "table"), + TestResult("ephemeral_true", TestStatus.Fail, None), + TestResult("ephemeral_false", TestStatus.Fail, None), + TestResult("ephemeral_unset", TestStatus.Fail, None), TestResult("unset_true", TestStatus.Fail, "table"), TestResult("unset_false", TestStatus.Fail, None), TestResult("unset_unset", TestStatus.Fail, None), @@ -156,6 +150,8 @@ class StoreTestFailuresAsProjectLevelOff(StoreTestFailuresAsBase): then store the failures in a view. - If `store_failures = False` in the project and `store_failures_as = "table"` in the model, then store the failures in a table. + - If `store_failures = False` in the project and `store_failures_as = "ephemeral"` in the model, + then do not store the failures. - If `store_failures = False` in the project and `store_failures_as` is not set, then do not store the failures. """ @@ -163,9 +159,10 @@ class StoreTestFailuresAsProjectLevelOff(StoreTestFailuresAsBase): @pytest.fixture(scope="class") def tests(self): return { - "results_view.sql": TEST__VIEW_UNSET, - "results_table.sql": TEST__TABLE_UNSET, - "results_unset.sql": TEST__UNSET_UNSET, + "results_view.sql": _files.TEST__VIEW_UNSET, + "results_table.sql": _files.TEST__TABLE_UNSET, + "results_ephemeral.sql": _files.TEST__EPHEMERAL_UNSET, + "results_unset.sql": _files.TEST__UNSET_UNSET, } @pytest.fixture(scope="class") @@ -176,6 +173,7 @@ def test_tests_run_successfully_and_are_stored_as_expected(self, project): expected_results = { TestResult("results_view", TestStatus.Fail, "view"), TestResult("results_table", TestStatus.Fail, "table"), + TestResult("results_ephemeral", TestStatus.Fail, None), TestResult("results_unset", TestStatus.Fail, None), } self.run_and_assert(project, expected_results) @@ -186,9 +184,6 @@ class StoreTestFailuresAsProjectLevelView(StoreTestFailuresAsBase): These scenarios test that `store_failures_as` at the project level takes precedence over `store_failures` at the model level. - Additionally, the fourth scenario demonstrates how to turn off `store_failures` at the model level - when `store_failures_as` is used at the project level. - Test Scenarios: - If `store_failures_as = "view"` in the project and `store_failures = False` in the model, @@ -197,17 +192,14 @@ class StoreTestFailuresAsProjectLevelView(StoreTestFailuresAsBase): then store the failures in a view. - If `store_failures_as = "view"` in the project and `store_failures` is not set, then store the failures in a view. - - If `store_failures_as = "view"` in the project and `store_failures = False` in the model - and `store_failures_as = None` in the model, then do not store the failures. """ @pytest.fixture(scope="class") def tests(self): return { - "results_true.sql": TEST__VIEW_TRUE, - "results_false.sql": TEST__VIEW_FALSE, - "results_unset.sql": TEST__VIEW_UNSET, - "results_turn_off.sql": TEST__NONE_FALSE, + "results_true.sql": _files.TEST__VIEW_TRUE, + "results_false.sql": _files.TEST__VIEW_FALSE, + "results_unset.sql": _files.TEST__VIEW_UNSET, } @pytest.fixture(scope="class") @@ -219,7 +211,44 @@ def test_tests_run_successfully_and_are_stored_as_expected(self, project): TestResult("results_true", TestStatus.Fail, "view"), TestResult("results_false", TestStatus.Fail, "view"), TestResult("results_unset", TestStatus.Fail, "view"), - TestResult("results_turn_off", TestStatus.Fail, None), + } + self.run_and_assert(project, expected_results) + + +class StoreTestFailuresAsProjectLevelEphemeral(StoreTestFailuresAsBase): + """ + This scenario tests that `store_failures_as` at the project level takes precedence over `store_failures` + at the model level. In particular, setting `store_failures_as = "ephemeral"` at the project level + turns off `store_failures` regardless of the setting of `store_failures` anywhere. Turning `store_failures` + back on at the model level requires `store_failures_as` to be set at the model level. + + Test Scenarios: + + - If `store_failures_as = "ephemeral"` in the project and `store_failures = True` in the project, + then do not store the failures. + - If `store_failures_as = "ephemeral"` in the project and `store_failures = True` in the project and the model, + then do not store the failures. + - If `store_failures_as = "ephemeral"` in the project and `store_failures_as = "view"` in the model, + then store the failures in a view. + """ + + @pytest.fixture(scope="class") + def tests(self): + return { + "results_unset.sql": _files.TEST__UNSET_UNSET, + "results_true.sql": _files.TEST__UNSET_TRUE, + "results_view.sql": _files.TEST__VIEW_UNSET, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return {"tests": {"store_failures_as": "ephemeral", "store_failures": True}} + + def test_tests_run_successfully_and_are_stored_as_expected(self, project): + expected_results = { + TestResult("results_unset", TestStatus.Fail, None), + TestResult("results_true", TestStatus.Fail, None), + TestResult("results_view", TestStatus.Fail, "view"), } self.run_and_assert(project, expected_results) @@ -235,8 +264,8 @@ class StoreTestFailuresAsGeneric(StoreTestFailuresAsBase): @pytest.fixture(scope="class") def models(self): return { - f"{self.model_table}.sql": MODEL__CHIPMUNKS, - "schema.yml": SCHEMA_YML, + f"{self.model_table}.sql": _files.MODEL__CHIPMUNKS, + "schema.yml": _files.SCHEMA_YML, } def test_tests_run_successfully_and_are_stored_as_expected(self, project): diff --git a/tests/functional/store_test_failures/test_store_test_failures.py b/tests/functional/store_test_failures/test_store_test_failures.py index 658adde116c..39d2e08dc9f 100644 --- a/tests/functional/store_test_failures/test_store_test_failures.py +++ b/tests/functional/store_test_failures/test_store_test_failures.py @@ -4,11 +4,14 @@ StoreTestFailuresAsInteractions, StoreTestFailuresAsProjectLevelOff, StoreTestFailuresAsProjectLevelView, + StoreTestFailuresAsProjectLevelEphemeral, StoreTestFailuresAsGeneric, ) -class TestStoreTestFailuresAsInteractions(StoreTestFailuresAsInteractions): +class PostgresMixin: + audit_schema: str + @pytest.fixture(scope="function", autouse=True) def setup_audit_schema(self, project, setup_method): # postgres only supports schema names of 63 characters @@ -16,25 +19,23 @@ def setup_audit_schema(self, project, setup_method): self.audit_schema = self.audit_schema[:63] -class TestStoreTestFailuresAsProjectLevelOff(StoreTestFailuresAsProjectLevelOff): - @pytest.fixture(scope="function", autouse=True) - def setup_audit_schema(self, project, setup_method): - # postgres only supports schema names of 63 characters - # a schema with a longer name still gets created, but the name gets truncated - self.audit_schema = self.audit_schema[:63] +class TestStoreTestFailuresAsInteractions(StoreTestFailuresAsInteractions, PostgresMixin): + pass -class TestStoreTestFailuresAsProjectLevelView(StoreTestFailuresAsProjectLevelView): - @pytest.fixture(scope="function", autouse=True) - def setup_audit_schema(self, project, setup_method): - # postgres only supports schema names of 63 characters - # a schema with a longer name still gets created, but the name gets truncated - self.audit_schema = self.audit_schema[:63] +class TestStoreTestFailuresAsProjectLevelOff(StoreTestFailuresAsProjectLevelOff, PostgresMixin): + pass -class TestStoreTestFailuresAsGeneric(StoreTestFailuresAsGeneric): - @pytest.fixture(scope="function", autouse=True) - def setup_audit_schema(self, project, setup_method): - # postgres only supports schema names of 63 characters - # a schema with a longer name still gets created, but the name gets truncated - self.audit_schema = self.audit_schema[:63] +class TestStoreTestFailuresAsProjectLevelView(StoreTestFailuresAsProjectLevelView, PostgresMixin): + pass + + +class TestStoreTestFailuresAsProjectLevelEphemeral( + StoreTestFailuresAsProjectLevelEphemeral, PostgresMixin +): + pass + + +class TestStoreTestFailuresAsGeneric(StoreTestFailuresAsGeneric, PostgresMixin): + pass