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

[CT-2320] [Regression] [v1.5] dbt list no longer suppresses info-level logging #7206

Closed
2 tasks done
Tracked by #7162
jtcohen6 opened this issue Mar 22, 2023 · 4 comments
Closed
2 tasks done
Tracked by #7162
Labels
bug Something isn't working logging regression

Comments

@jtcohen6
Copy link
Contributor

jtcohen6 commented Mar 22, 2023

Is this a regression in a recent version of dbt-core?

  • I believe this is a regression in dbt-core functionality
  • I have searched the existing issues, and I could not find an existing issue for this regression

Current Behavior

Using latest beta / main branch:

$ dbt ls
13:47:10  Running with dbt=1.5.0-b4
13:47:10  Unable to do partial parsing because of a version mismatch
13:47:11  Found 2 models, 0 tests, 0 snapshots, 0 analyses, 310 macros, 0 operations, 1 seed file, 0 sources, 0 exposures, 0 metrics, 1 group
test.subdir.another_model
test.my_model
test.my_seed
(env) 14:47:11 ~/d

Expected/Previous Behavior

v1.4.x:

$ dbt ls
test.subdir.another_model
test.my_model
test.my_seed

This is nice because it allows users to easily pipe the results (and only the results) to jq / a file!

Steps To Reproduce

  1. Use latest main
  2. Run dbt ls
  3. See that info-level logging is included in stdout

Relevant log output

No response

Environment

- dbt (working version): v1.4
- dbt (regression version): v1.5 (beta)

Which database adapter are you using with dbt?

No response

Additional Context

Previous logic in 1.4.latest branch:

@classmethod
def pre_init_hook(cls, args):
"""A hook called before the task is initialized."""
# Filter out all INFO-level logging to allow piping ls output to jq, etc
# WARN level will still include all warnings + errors
# Do this by:
# - returning the log level so that we can pass it into the 'level_override'
# arg of events.functions.setup_event_logger() -- good!
# - mutating the initialized, not-yet-configured STDOUT event logger
# because it's being configured too late -- bad! TODO refactor!
log_manager.stderr_console()
super().pre_init_hook(args)
return EventLevel.WARN

This logic was removed in #6541.

FWIW it's possible to achieve the previous behavior, using v1.5, thanks to the new --log-level flag:

$ dbt --log-level warn ls
test.subdir.another_model
test.my_model
test.my_seed

Acceptance criteria:

  • Make --log-level warn the default behavior with list, without needing to pass the --log-level flag
  • If a user does explicitly pass/set the --log-level flag (or env var), it should take precedence over the default used for list
@github-actions github-actions bot changed the title [Regression] [v1.5] dbt list no longer suppresses info-level logging [CT-2320] [Regression] [v1.5] dbt list no longer suppresses info-level logging Mar 22, 2023
@jtcohen6
Copy link
Contributor Author

Another valid workaround:

$ dbt --quiet ls
test.subdir.another_model
test.my_model
test.my_seed

@iknox-fa
Copy link
Contributor

Per BLG Mar 27 2023: We're pointing this ticket as is, however the inconsistent use of a default log level feels a little dodgy. @jtcohen6 should we have a larger conversation about default log levels and expected outputs at some point?

@jtcohen6
Copy link
Contributor Author

@iknox-fa I agree it's a little dodgy :)

My primary motivation here is in restoring the previous default behavior, since this wasn't a change we'd intended. If that's too tricky, though, we can document the workarounds of --log-level warn or --quiet to achieve the same behavior as the previous default.

@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Mar 29, 2023

After chatting about this with more folks internally, I've come around to:

  • This was weird & inconsistent behavior before (not to mention, poorly defined & documented)
  • It's a defensible change, as part of our move toward a more consistent & solid application
  • There are such easy & sensible mechanisms to achieve exactly the same behavior as before, it just requires passing an explicit flag instead of relying on the implicit/magical inconsistency

I'm going to close this issue and kick this out of scope for v1.5, and open a docs issue to make sure this is called out within the "Breaking Changes" section of the v1.5 migration guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working logging regression
Projects
None yet
Development

No branches or pull requests

2 participants