-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add summary output for dbt source freshness command #12231
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
base: main
Are you sure you want to change the base?
Add summary output for dbt source freshness command #12231
Conversation
…mmary output for dbt source freshness command Fixes dbt-labs#6248 Add summary output to `dbt source freshness` command showing errors and warnings with final statistics, similar to other dbt commands like `dbt test` and `dbt run`. Changes: - Modified task_end_messages() in freshness.py to collect and display errors/warnings - Added summary header showing count of errors and warnings - Print each error/warning with source and table name - Added final statistics line (PASS/WARN/ERROR/SKIP/TOTAL) - Added Formatting import from dbt_common.events.types This implementation only modifies freshness-related code as requested by maintainers.
|
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @kalluripradeep |
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
|
Great feature! Could you add unit tests for task_end_messages() covering scenarios like: all pass, some warnings, some errors, mixed results, and empty results? |
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 @kalluripradeep for this contribution. I've been waiting for this UX improvement. Ultimate blessing for BI/data teams monitoring source freshness.
What I liked:
- Clean implementation following maintainer's guidance
- Good use of existing event patterns (
fire_event,Note,Formatting) - Proper pluralization in the summary message
Questions/Suggestions:
- Could you clarify why
RunStatus.Errorwas removed from the error handling? - Are there plans to add unit tests for the new
task_end_messages()behavior? - Minor: Consider extracting the error/warning print logic into a helper to reduce duplication
Overall, LGTM!
| ): | ||
| print_run_result_error(result) | ||
|
|
||
| if result.status in (FreshnessStatus.Error, FreshnessStatus.RuntimeErr): |
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.
I noticed RunStatus.Error was previously handled but is now excluded. Is this intentional? Could there be scenarios where a freshness check returns RunStatus.Error?
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.
I removed RunStatus.Error because freshness checks return FreshnessStatus (Error, RuntimeErr, Warn, Pass), not RunStatus. The freshness task has its own status enum separate from run/test tasks. I've verified this covers all freshness error scenarios.
core/dbt/task/freshness.py
Outdated
| EventLevel.INFO, | ||
| ) | ||
|
|
||
| # Print each error |
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.
Minor suggestion: The error and warning printing logic is quite similar. Consider extracting a helper function like _print_result_message(result, level, prefix) to reduce duplication.
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.
I'll extract a helper function _print_freshness_result() to reduce the duplication between error and warning printing.
core/dbt/task/freshness.py
Outdated
| # Print each error | ||
| for error in errors: | ||
| fire_event(Formatting("")) | ||
| if hasattr(error, 'node'): |
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.
The hasattr checks are good, but what happens if neither error.node nor error.source_name/error.table_name exist? Consider adding an else fallback or a comment explaining when each case occurs.
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.
I'll add an else fallback. The error.node case handles standard freshness results, while source_name/table_name handle edge cases. I'll add a comment explaining this and a fallback for unexpected cases.
Absolutely! I'll add unit tests covering:
Working on this now. |
Thanks for the feedback @pizofreude! I've addressed all your suggestions:
All changes pushed and ready for review! |
Description
Fixes #6248
This PR adds summary output to the
dbt source freshnesscommand to display errors and warnings at the end of execution, similar to other dbt commands likedbt testanddbt run.Previously, when running
dbt source freshness, errors and warnings would get lost in the output with no summary. This made it difficult to quickly identify which sources had issues, especially when checking many sources.Changes
task_end_messages()incore/dbt/task/freshness.pyto collect and display errors/warningsFormattingimport fromdbt_common.events.typesImplementation Details
This implementation follows the maintainer's guidance to only modify freshness-related code and not touch other run result handling. The solution is self-contained within the freshness task.
Example Output
After this change, when running
dbt source freshness, users will see:Completed with 1 error and 1 warning:
Failure in source my_source.stale_table
Warning in source my_source.warn_table
Done. PASS=1 WARN=1 ERROR=1 SKIP=0 TOTAL=3
Testing
task/printer.pyChecklist