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 skip criteria (#1093) #1094

Merged
merged 3 commits into from
Oct 29, 2018

Conversation

beckjake
Copy link
Contributor

Fixes #1093

On nodes skipped due to failures, only log ERROR SKIP when the skip cause node was ephemeral.

@drewbanin
Copy link
Contributor

drewbanin commented Oct 25, 2018

@beckjake this behavior looks great! Can we update the error message a little? Right now it shows:

$ dbt run --model broken+
Found 10 models, 1 tests, 0 archives, 0 analyses, 96 macros, 4 operations, 1 seed files

13:37:29 | Concurrency: 8 threads (target='dev')
13:37:29 |
13:37:29 | 1 of 1 SKIP relation demo_schema.jerco due to ephemeral model error.. [ERROR SKIP]
Compilation Error in model broken (models/broken.sql)
  'reg' is undefined
13:37:29 |
13:37:29 | Finished running 1 view models in 0.28s.

Completed with 1 errors:

Compilation Error in referenced ephemeral model demo_schema.broken

I think it would be great if the last line could read:

Compilation Error in <model.package.name>, caused by compilation error in referenced ephemeral model <model.package.name>

Where the first <model.package.name> is the model that ref's the ephemeral model, and the second <model.package.name> is the ephemeral model itself.

The current output of Compilation Error in referenced ephemeral model demo_schema.broken is a little confusing because it 1) does not show you the model that failed to run and 2) it presents the ephemeral model as project.model, which looks like schema.table, which is not a thing for ephemeral models. Happy to discuss!

@beckjake
Copy link
Contributor Author

beckjake commented Oct 25, 2018

@drewbanin resolved, I think:

$ dbt run --models broken+
Found 6 models, 0 tests, 0 archives, 0 analyses, 94 macros, 2 operations, 1 seed files

12:02:37 | Concurrency: 2 threads (target='default')
12:02:37 |
12:02:37 | 1 of 1 SKIP relation dbt_postgres.jerco due to ephemeral model error. [ERROR SKIP]
Compilation Error in model broken (models/broken.sql)
  'reg' is undefined
12:02:37 |
12:02:37 | Finished running 1 view models in 0.24s.

Completed with 1 errors:

Compilation Error in model.talk.broken, caused by compilation error in referenced ephemeral model model.talk.jerco

Done. PASS=0 ERROR=1 SKIP=0 TOTAL=1

@beckjake
Copy link
Contributor Author

And with non-ephemerals:

Found 6 models, 0 tests, 0 archives, 0 analyses, 94 macros, 2 operations, 1 seed files

12:04:42 | Concurrency: 2 threads (target='default')
12:04:42 |
12:04:42 | 1 of 2 START view model dbt_postgres.broken2......................... [RUN]
12:04:42 | 1 of 2 ERROR creating view model dbt_postgres.broken2................ [ERROR in 0.04s]
12:04:42 | 2 of 2 SKIP relation dbt_postgres.jerco2............................. [SKIP]
12:04:42 |
12:04:42 | Finished running 2 view models in 0.24s.

Completed with 1 errors:

Database Error in model broken2 (models/broken2.sql)
  syntax error at or near "1"
  LINE 2:     select 1 1 1
                       ^
  compiled SQL at target/compiled/talk/broken2.sql

Done. PASS=0 ERROR=1 SKIP=1 TOTAL=2

@drewbanin
Copy link
Contributor

drewbanin commented Oct 25, 2018

I think these model uids might be backwards!

Compilation Error in model.talk.broken, caused by compilation error in referenced ephemeral model model.talk.jerco

Copy link
Contributor

@drewbanin drewbanin left a comment

Choose a reason for hiding this comment

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

Nice! Tested this a bunch, and it works great. Ship it

@beckjake beckjake merged commit 37a888d into dev/guion-bluford Oct 29, 2018
@beckjake beckjake deleted the fix/only-error-skip-on-ephemeral-failures branch October 29, 2018 14:17
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