-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Feature: defer tests #2954
Feature: defer tests #2954
Conversation
This comment has been minimized.
This comment has been minimized.
6a27371
to
9d87535
Compare
This comment has been minimized.
This comment has been minimized.
9d87535
to
8ed1f70
Compare
I like this a lot, and I think it's such a fascinating reframing of the original problem. I buy the argument you're laying out here, and am highly supportive of our answer for CI being:
instead of
I think I need to go deeper on some of these deferral mechanics, but I wonder if there are new UX-things we should be doing to help clarify what, actually, is happening in a given model/test execution. That could very well be a topic for consideration in the "Improve testing UX" conversation. Just flagging that dbt's CLI output used to map 1-to-1 with objects created or tested in the database, but now, a given test could actually run queries that join across dev and prod schemas. That's A Feature, but I do want to make sure that we're appropriately representing it to users and creating the opportunity for effective debugging if things go wrong. Just throwing it out there, happy to discuss further or shelf it for the future. For the moment though, really like this approach |
8ed1f70
to
274c301
Compare
This is a really fair point. Currently, we have a debug-level log message to enumerate which nodes have been deferred:
We could update the wording, and make it log to stdout instead. |
Yeah - I'm almost picturing that those nodes show up in the stdout logs as though we were running them, but they have a status like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Thanks for the review, all. I spent a decent chunk of time today rewriting docs to account for these functional changes (dbt-labs/docs.getdbt.com#488). Separately, let's continue discussing cosmetic changes:
|
@jtcohen6 you want to open up a new issue for those discussions? Feels like they're important ones, and probably doesn't make sense to relegate them to the bottom of this PR! |
resolves #2701
picks up from #2706
requires #2946
Description
There's a lengthy section of the state comparison caveats devoted solely to tests. To make a long story short:
dbt run -m state:modified && dbt test -m state:modified
will return a failure because we will execute the test without having built its parentPreviously, I have recommended that community members use the advanced node selection syntax available in v0.18 to pick only the tests they'd want to run (
--exclude test_name:relationships test_type:data
), or to ensure that the parent models of modified tests were always built with commands like:In truth, everything would be a lot simpler if we added support for
dbt test --defer
. I believe we can do that, following a subtle-but-significant change to what deferral actually means (#2946). If a parent resource (seed, model, snapshot, etc) exists in the current namespace, we use it; if it doesn't exist in the current namespace, we defer its reference to the state manifest's namespace. (Inclusion or exclusion from the selection criteria has nothing to do with which objects are deferred, since models are never included by the criteria ofdbt test
.)Let's say we're running a CI job, and comparing against a manifest from the main branch / production. Our
model_b
has been changed,model_a
has not, and there's arelationships
test between them.Our CI job can now be very simple:
In the first step, we will build
ci_schema.model_b
because it has been modified. In the second step, we will execute therelationships
test, which is selected because its parentmodel_b
is modified. The compiled SQL for that test will look like:Granted, I don't think
relationships
tests make a lot of sense in a CI context, and they make even less sense when trying to assert referential integrity across environments. That being said, the query will succeed and return a number of rows. It's then up to the user to decide if they want to update their selection criteria with--exclude test_name:relationships
.Checklist
CHANGELOG.md
and added information about my change to the "dbt next" section.