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

Fix error handling in dbt build #3608

Merged
merged 2 commits into from
Jul 26, 2021
Merged

Conversation

jtcohen6
Copy link
Contributor

@jtcohen6 jtcohen6 commented Jul 21, 2021

resolves #3598

Description

I saw that the BuildTask was inheriting from CompileTask, which sets:
https:/dbt-labs/dbt/blob/9f716b31b3e0ca57a3722809f8c3405a7b5752f8/core/dbt/task/compile.py#L36-L38

That's the cause of the weird behavior I was seeing when reviewing #3490. We could just override that method for the BuildTask, but what I think we really want is for dbt build to operate at parity with dbt run, and take all of its cues—including how it handles hooks, deferral, etc, etc. I confirmed that TestTask, SeedTask, SnapshotTask also inherit from RunTask, so it feels appropriate that BuildTask would as well. Let me know if I'm totally off base here.

Tiny thing: I confirmed that a failed model will be handled and downstream resources will be skipped. Now that tests can be skipped, in addition to models/snapshots, I figured we should tweak the SKIP log message to be more inclusive of tests.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

@jtcohen6 jtcohen6 requested a review from iknox-fa July 21, 2021 19:45
@jtcohen6 jtcohen6 temporarily deployed to Postgres July 21, 2021 19:45 Inactive
@cla-bot cla-bot bot added the cla:yes label Jul 21, 2021
@jtcohen6 jtcohen6 temporarily deployed to Bigquery July 21, 2021 19:45 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery July 21, 2021 19:45 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift July 21, 2021 19:45 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift July 21, 2021 19:45 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake July 21, 2021 19:45 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake July 21, 2021 19:45 Inactive
@iknox-fa
Copy link
Contributor

Good catch! I think I chose to inherit from CompileTask ealy on when I thought I needed the selection spec stuff found there.

@jtcohen6 jtcohen6 force-pushed the fix/dbt-build-error-handling branch from 6440392 to d7c6d31 Compare July 26, 2021 01:58
@jtcohen6 jtcohen6 temporarily deployed to Snowflake July 26, 2021 01:58 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake July 26, 2021 01:58 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery July 26, 2021 01:58 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery July 26, 2021 01:58 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift July 26, 2021 01:58 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift July 26, 2021 01:58 Inactive
@jtcohen6
Copy link
Contributor Author

Some failing tests, but they all look related to the flakey issues we're currently tackling elsewhere, and unrelated to the changes in this PR.

@jtcohen6 jtcohen6 merged commit 7272263 into develop Jul 26, 2021
@jtcohen6 jtcohen6 deleted the fix/dbt-build-error-handling branch July 26, 2021 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dbt build: Correct non-fast-failing behavior to match other tasks such as run
2 participants