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

ADAP-850: Support test results as a view #8653

Merged
merged 40 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
718b892
add strategy parameter to TestConfig, default to ephemeral, catch str…
mikealfare Sep 15, 2023
d4dfec8
changie
mikealfare Sep 15, 2023
e999ed2
create test results as views
mikealfare Sep 16, 2023
f5e0797
removed unnecessary formatting changes
mikealfare Sep 16, 2023
8e366bb
removed unnecessary formatting changes
mikealfare Sep 16, 2023
1a3bd81
removed unnecessary formatting changes
mikealfare Sep 16, 2023
7b4be33
removed unnecessary formatting changes
mikealfare Sep 16, 2023
a149d66
removed unnecessary formatting changes
mikealfare Sep 16, 2023
0313904
removed unnecessary formatting changes
mikealfare Sep 16, 2023
ccf199c
removed unnecessary formatting changes
mikealfare Sep 16, 2023
f76fc74
removed unnecessary formatting changes
mikealfare Sep 16, 2023
9a0656f
updated test expected values for new config option
mikealfare Sep 16, 2023
40b646b
break up tests into reusable tests and adapter specific configuration…
mikealfare Sep 16, 2023
661c5f6
use built-in utility to test relation types, reduce configuration to …
mikealfare Sep 21, 2023
3fa0858
move configuration into base test class
mikealfare Sep 21, 2023
49b797e
separate setup and teardown methods, move postgres piece back into db…
mikealfare Sep 21, 2023
069a9b2
shorten inserted record values to avoid data type issues in the model…
mikealfare Sep 21, 2023
61d514b
shorten inserted record values to avoid data type issues in the model…
mikealfare Sep 21, 2023
beb0ed4
add audit schema suffix as class attribute
mikealfare Sep 22, 2023
5aa2031
rename `strategy` parameter to `store_failures_as`; allow `store_fail…
mikealfare Sep 29, 2023
9971be7
updated changelog entry to reflect correct parameters
mikealfare Sep 29, 2023
3ddc7d4
updated changelog entry to reflect correct parameters
mikealfare Sep 29, 2023
4b81e22
revert unexpected formatting changes from black
mikealfare Sep 29, 2023
0833774
revert test debugging artifacts
mikealfare Sep 29, 2023
eb1f3b4
update local variable to be more intuitive
mikealfare Sep 29, 2023
cf86565
update default for store_test_failures_as; rename postgres test to re…
mikealfare Sep 29, 2023
2f1aa91
remove duplicated default for store_failures_as
mikealfare Sep 29, 2023
96d18a9
update expected test config dicts to include the new default value fo…
mikealfare Sep 29, 2023
c214595
Merge branch 'main' into feature/materialized-tests/adap-850
mikealfare Sep 29, 2023
0baf090
Add `store_failures_as` config for generic tests
dbeatty10 Sep 30, 2023
1b1d9c6
revert code to prioritize store_failures_as over store_failures
mikealfare Oct 3, 2023
6c93e34
cover --store-failures on CLI gap
mikealfare Oct 3, 2023
a47e59e
add generic tests test case for store_failures_as
mikealfare Oct 3, 2023
86b2793
Merge branch 'main' into feature/materialized-tests/adap-850
mikealfare Oct 3, 2023
90792e0
Merge branch 'dbeatty/config-test-materializations-2' into feature/ma…
mikealfare Oct 3, 2023
c09b81d
update object names for generic test case tests for store_failures_as
mikealfare Oct 3, 2023
729d939
remove unique generic test, it was not testing `store_failures_as`
mikealfare Oct 4, 2023
9d3fb06
pull generic run and assertion into base test class to turn tests int…
mikealfare Oct 4, 2023
e70dca3
add ephemeral option for store_failures_as, as a way to easily turn o…
mikealfare Oct 9, 2023
8b8f9bb
add compilation error for invalid setting of store_failures_as
mikealfare Oct 9, 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-20230915-174428.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: Support storing test failures as views
time: 2023-09-15T17:44:28.833877-04:00
custom:
Author: mikealfare
Issue: "6914"
51 changes: 50 additions & 1 deletion core/dbt/contracts/graph/model_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ class Hook(dbtClassMixin, Replaceable):

@dataclass
class BaseConfig(AdditionalPropertiesAllowed, Replaceable):

# enable syntax like: config['key']
def __getitem__(self, key):
return self.get(key)
Expand Down Expand Up @@ -555,12 +554,61 @@ class TestConfig(NodeAndTestConfig):
# Annotated is used by mashumaro for jsonschema generation
severity: Annotated[Severity, Pattern(SEVERITY_PATTERN)] = Severity("ERROR")
store_failures: Optional[bool] = None
store_failures_as: Optional[str] = None
where: Optional[str] = None
limit: Optional[int] = None
fail_calc: str = "count(*)"
warn_if: str = "!= 0"
error_if: str = "!= 0"

def __post_init__(self):
"""
The presence of a setting for `store_failures_as` overrides any existing setting for `store_failures`,
regardless of level of granularity. If `store_failures_as` is not set, then `store_failures` takes effect.
At the time of implementation, `store_failures = True` would always create a table; the user could not
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 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

"""

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

# 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:
"""This is like __eq__, except it explicitly checks certain fields."""
Expand All @@ -572,6 +620,7 @@ def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> boo
"warn_if",
"error_if",
"store_failures",
"store_failures_as",
]

seen = set()
Expand Down
1 change: 1 addition & 0 deletions core/dbt/contracts/relation.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class RelationType(StrEnum):
CTE = "cte"
MaterializedView = "materialized_view"
External = "external"
Ephemeral = "ephemeral"


class ComponentName(StrEnum):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,27 @@

{% 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') %}
-- 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='table') -%} %}
identifier=identifier, schema=schema, database=database, type=store_failures_as) -%} %}

{% if old_relation %}
{% do adapter.drop_relation(old_relation) %}
{% endif %}

{% call statement(auto_begin=True) %}
{{ create_table_as(False, target_relation, sql) }}
{{ get_create_sql(target_relation, sql) }}
mikealfare marked this conversation as resolved.
Show resolved Hide resolved
{% endcall %}

{% do relations.append(target_relation) %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@

{%- macro default__get_create_sql(relation, sql) -%}

{%- if relation.is_materialized_view -%}
{%- if relation.is_view -%}
{{ get_create_view_as_sql(relation, sql) }}

{%- elif relation.is_table -%}
{{ get_create_table_as_sql(False, relation, sql) }}

{%- elif relation.is_materialized_view -%}
{{ get_create_materialized_view_as_sql(relation, sql) }}

{%- else -%}
Expand Down
7 changes: 7 additions & 0 deletions core/dbt/parser/generic_test_builders.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
"error_if",
"fail_calc",
"store_failures",
"store_failures_as",
"meta",
"database",
"schema",
Expand Down Expand Up @@ -242,6 +243,10 @@
def store_failures(self) -> Optional[bool]:
return self.config.get("store_failures")

@property
def store_failures_as(self) -> Optional[bool]:
return self.config.get("store_failures_as")

@property
def where(self) -> Optional[str]:
return self.config.get("where")
Expand Down Expand Up @@ -294,6 +299,8 @@
config["fail_calc"] = self.fail_calc
if self.store_failures is not None:
config["store_failures"] = self.store_failures
if self.store_failures_as is not None:
config["store_failures_as"] = self.store_failures_as

Check warning on line 303 in core/dbt/parser/generic_test_builders.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/generic_test_builders.py#L303

Added line #L303 was not covered by tests
if self.meta is not None:
config["meta"] = self.meta
if self.database is not None:
Expand Down
150 changes: 150 additions & 0 deletions tests/adapter/dbt/tests/adapter/store_test_failures_tests/_files.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
SEED__CHIPMUNKS = """
name,shirt
alvin,red
simon,blue
theodore,green
dave,
""".strip()


MODEL__CHIPMUNKS = """
{{ config(materialized='table') }}
select *
from {{ ref('chipmunks_stage') }}
"""


TEST__VIEW_TRUE = """
{{ config(store_failures_as="view", store_failures=True) }}
select *
from {{ ref('chipmunks') }}
where shirt = 'green'
"""


TEST__VIEW_FALSE = """
{{ config(store_failures_as="view", store_failures=False) }}
select *
from {{ ref('chipmunks') }}
where shirt = 'green'
"""


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


TEST__TABLE_TRUE = """
{{ config(store_failures_as="table", store_failures=True) }}
select *
from {{ ref('chipmunks') }}
where shirt = 'green'
"""


TEST__TABLE_FALSE = """
{{ config(store_failures_as="table", store_failures=False) }}
select *
from {{ ref('chipmunks') }}
where shirt = 'green'
"""


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


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 *
from {{ ref('chipmunks') }}
where shirt = 'green'
"""


TEST__UNSET_FALSE = """
{{ config(store_failures=False) }}
select *
from {{ ref('chipmunks') }}
where shirt = 'green'
"""


TEST__UNSET_UNSET = """
select *
from {{ ref('chipmunks') }}
where shirt = 'green'
"""


TEST__VIEW_UNSET_PASS = """
{{ config(store_failures_as="view") }}
select *
from {{ ref('chipmunks') }}
where shirt = 'purple'
"""


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


SCHEMA_YML = """
version: 2

models:
- name: chipmunks
columns:
- name: name
tests:
- not_null:
store_failures_as: view
- accepted_values:
store_failures: false
store_failures_as: table
values:
- alvin
- simon
- theodore
- name: shirt
tests:
- not_null:
store_failures: true
store_failures_as: view
"""
Loading
Loading