select asset checks with build_dbt_asset_selection #18098
Conversation
johannkm
force-pushed
the
johann/11-16-select_asset_checks_with_build_dbt_asset_selection
branch
from
95da572 to
d898be4
Compare
|
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
johannkm
force-pushed
the
johann/11-16-select_asset_checks_with_build_dbt_asset_selection
branch
from
d898be4 to
9b47619
Compare
| def test_selection_customers(): | ||
| asset_selection = build_dbt_asset_selection([my_dbt_assets], dbt_select="customers") | ||
| assert asset_selection.resolve(asset_graph) == {AssetKey(["customers"])} | ||
| # all tests that reference model customers- includes a relationship test on orders |
There was a problem hiding this comment.
I'm a little surprised by this, am I interpreting correctly?
There was a problem hiding this comment.
The default mode is eager, so this matches that behavior. Perhaps we should augment build_dbt_asset_selection to include indirect_selection as a parameter?
There was a problem hiding this comment.
Yeah seems like a likely request in the future
python_modules/libraries/dagster-dbt/dagster_dbt/dbt_manifest_asset_selection.py
Outdated
Show resolved
Hide resolved
python_modules/libraries/dagster-dbt/dagster_dbt/dbt_manifest_asset_selection.py
Outdated
Show resolved
Hide resolved
python_modules/libraries/dagster-dbt/dagster_dbt_tests/core/test_asset_check_selection.py
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
|
|
||
| def test_all(): |
There was a problem hiding this comment.
Any reason why we're not parametrizing this? All the tests essentially have the same assertion.
There was a problem hiding this comment.
IMO repeating a couple assert lines is easier to read and adjust than parameterizing test input, output, and name. Parameterizing makes sense for massive suites like AMP but I like keeping it simple when we can
johannkm
force-pushed
the
johann/11-16-select_asset_checks_with_build_dbt_asset_selection
branch
from
dbf3745 to
610e9fa
Compare
|
Deploy preview for dagster-university ready! ✅ Preview Built with commit 610e9fa. |
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 610e9fa. |
johannkm
deleted the
johann/11-16-select_asset_checks_with_build_dbt_asset_selection
branch
Support things like
Previously, the selection would include any asset checks that targeted whatever dbt assets were selected. Now we rely on dbt to do that selection for us. Since we're hard coded to eager it's usually pretty similar, though there's a difference with relationship tests that I point out in a comment.
Another niche change is that previously this would select a non-dbt AssetCheck that targeted a selected dbt asset. I think that's a welcome change.
Both are minor breaking changes to an experimental feature that I don't think anyone will hit.
Closes #17372