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

dbt --warn-error does not error when no models are selected #4006

Closed
1 of 5 tasks
laxjesse opened this issue Oct 5, 2021 · 3 comments · Fixed by #4019
Closed
1 of 5 tasks

dbt --warn-error does not error when no models are selected #4006

laxjesse opened this issue Oct 5, 2021 · 3 comments · Fixed by #4019
Labels
bug Something isn't working good_first_issue Straightforward + self-contained changes, good for new contributors!

Comments

@laxjesse
Copy link
Contributor

laxjesse commented Oct 5, 2021

Describe the bug

running dbt --warn-error run with selection syntax that selects no models returns a warning and not an error.

Steps To Reproduce

dbt --warn-error run -m example --exclude example

tried with a number of different selection syntax as well, such as

 dbt --warn-error run -m example --exclude config.materialized:table

(example.sql a table)

both commands warn that there is nothing to do, but return exit status 0

Expected behavior

I would expect an error instead of a warning.

Screenshots and log output

Found 1997 models, 3441 tests, 11 snapshots, 6 analyses, 547 macros, 0 operations, 30 seed files, 782 sources, 0 exposures

WARNING: Nothing to do. Try checking your model configs and model specification args

System information

Which database are you using dbt with?

  • postgres
  • redshift
  • bigquery
  • snowflake
  • other (specify: ____________)

The output of dbt --version:

installed version: 0.20.1
   latest version: 0.21.0

Your version of dbt is out of date! You can find instructions for upgrading here:
https://docs.getdbt.com/docs/installation

Plugins:
  - snowflake: 0.20.1
  - redshift: 0.20.1
  - postgres: 0.20.1

Tested on version 0.21.0 as well ^

The operating system you're using:
Mac OS Big Sur

The output of python --version:
Python 3.6.8

Additional context

Add any other context about the problem here.

@laxjesse laxjesse added bug Something isn't working triage labels Oct 5, 2021
@jtcohen6
Copy link
Contributor

jtcohen6 commented Oct 6, 2021

@laxjesse Thanks for the issue! I'll share a bit more of my thought process below; to make a long story short, this is a quite simple change. This line:

https://github.com/dbt-labs/dbt/blob/b501f4317c8180ab259f456ab254b34f74d0d288/core/dbt/task/runnable.py#L441-L442

Should instead use exceptions.warn_or_error:

warn_or_error("\nWARNING: Nothing to do. Try checking your model " 
                "configs and model specification args") 

(You'll need to add warn_or_error to the exceptions import list at the top of the file, too.)

We should also make the equivalent change for the list task, by switching:

https://github.com/dbt-labs/dbt/blob/b501f4317c8180ab259f456ab254b34f74d0d288/core/dbt/task/list.py#L64

Following the comment in #2886:

--warn-error could be used to opt back in to the current behaviour.

Would you be interested in contributing those quick fixes? :)


Bigger question: Does it make sense for this to return a warning, or should it be info only? Reasons I'm asking:

I'm raising those in case you have thoughts there. When I look back at the history of the warn_error flag (#1280), it does seem like the explicit intention was:

makes everything that says "warning" go through warn_or_error

We only missed this warning message because, at the time of that PR, it was info-level. We switched it to warning level a month later (#1341).

Basically, we should never use logger.warning() in place of warn_or_error(). Beyond runnable and list tasks, I see one other prominent place where warning is used: in the tracking module. That seems right to me; a failure to track should not brick a dbt invocation, even one with --warn-error enabled.

@jtcohen6 jtcohen6 added good_first_issue Straightforward + self-contained changes, good for new contributors! and removed triage labels Oct 6, 2021
@laxjesse
Copy link
Contributor Author

laxjesse commented Oct 6, 2021

@jtcohen6 thanks for the info! I think I can try and make these changes

Does it make sense for this to return a warning, or should it be info only?

to speak to my specific use case, my intention was to use --warn-error in production situations since we have specific models/directories scheduled separately. in these cases I'd definitely want this to return an error.

conversely, your CI example does seem like a reasonable situation where you'd want the opposite behavior

I'd still say that yes this should error bc

  1. makes everything that says "warning" go through warn_or_error was my understanding as well
  2. I think it's better to force the CI use case to have to recognize and swallow the error rather than force the prod use case to parse a warning as an error (my opinion, I could be wrong)

@StevenReitsma
Copy link

For anyone else stumbling on this issue through search, you can include the following config if you don't want to error when no models are selected, but do want to be strict on other warnings in CI:

config:
  warn_error_options:
    include: all
    exclude:
      - NothingToDo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good_first_issue Straightforward + self-contained changes, good for new contributors!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants