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

feat(dbt): model dbt tests as asset checks only on their attached dbt node #16727

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

rexledesma
Copy link
Contributor

@rexledesma rexledesma commented Sep 22, 2023

Summary & Motivation

There are certain dbt tests that act on multiple models. For example:

Currently, for these tests, we emit an AssetCheckResult for each of the models that the test acts on.

However, this runs into an error in subsetting. When you run an individual asset that has this relationship test defined, an exception occurs because two AssetCheckResult's when you run this test. This does not match the expected targeted assets.

A sample error:

dagster._core.errors.DagsterInvariantViolationError: Received unexpected AssetCheckResult. It targets asset 'orders' which is not targeted by any of the checks currently being evaluated. Targeted assets: ['customers'].

Some solutions:

  1. We prevent relationship tests from being models as Dagster tests right now.
  2. We only model relationship tests on the dbt model that defines the relationship test. The other models that are referenced in the test will not receive an asset check.

In this change, we implement (2). Furthermore, we prevent singular tests from being modeled as dbt tests, as they are not currently defined on a node.

In the future, we can also model singular tests that only target a single dbt node reference.

How I Tested These Changes

pytest: update existing tests, add singular test and assert that it is not modeled as an asset check.

@rexledesma
Copy link
Contributor Author

rexledesma commented Sep 22, 2023

@rexledesma rexledesma force-pushed the rl/model-dbt-tests-on-attached-node branch from 7159b00 to 4d0242b Compare September 22, 2023 19:43
@rexledesma rexledesma force-pushed the rl/allow-subsetted-asset-checks branch from db87ca0 to 13dfb5f Compare September 25, 2023 13:20
@rexledesma rexledesma force-pushed the rl/model-dbt-tests-on-attached-node branch from 4d0242b to c666554 Compare September 25, 2023 13:20
@rexledesma rexledesma force-pushed the rl/allow-subsetted-asset-checks branch from 13dfb5f to ea29644 Compare September 25, 2023 13:28
@rexledesma rexledesma force-pushed the rl/model-dbt-tests-on-attached-node branch 2 times, most recently from 554fd4c to 3b1082b Compare September 25, 2023 14:30
@rexledesma rexledesma self-assigned this Sep 25, 2023
@rexledesma rexledesma marked this pull request as ready for review September 25, 2023 14:31
Copy link
Contributor

@PedramNavid PedramNavid left a comment

Choose a reason for hiding this comment

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

Nicely done

@johannkm johannkm force-pushed the rl/allow-subsetted-asset-checks branch 2 times, most recently from 0d3830a to d64839e Compare September 26, 2023 16:43
@rexledesma rexledesma force-pushed the rl/model-dbt-tests-on-attached-node branch from 3b1082b to 83b5e63 Compare September 26, 2023 18:03
@johannkm johannkm force-pushed the rl/allow-subsetted-asset-checks branch 8 times, most recently from dc2c0f5 to 6770d5d Compare September 26, 2023 20:16
@rexledesma rexledesma force-pushed the rl/allow-subsetted-asset-checks branch from 6770d5d to d405c22 Compare September 26, 2023 22:08
@rexledesma rexledesma force-pushed the rl/model-dbt-tests-on-attached-node branch from 83b5e63 to 228070a Compare September 26, 2023 22:08
@johannkm johannkm force-pushed the rl/allow-subsetted-asset-checks branch from d405c22 to dda4b01 Compare September 27, 2023 01:42
Base automatically changed from rl/allow-subsetted-asset-checks to master September 27, 2023 01:45
@rexledesma rexledesma force-pushed the rl/model-dbt-tests-on-attached-node branch from 228070a to 7674850 Compare September 27, 2023 01:54
@rexledesma rexledesma merged commit 5497d82 into master Sep 27, 2023
@rexledesma rexledesma deleted the rl/model-dbt-tests-on-attached-node branch September 27, 2023 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants