-
Notifications
You must be signed in to change notification settings - Fork 978
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
Model-level notifications #6218
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Added some comments and questions and tagged Rakesh where we can provide a better view as the tech lead here
At a high level, depending on the UI settings, users can choose to be notified on - The UI setting applies for both models and tests, we don't differentiate them in the backend. When users receive an email, the email will say whether it is for a model/test. |
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.
Small comments and then LGTM!
|
||
::: | ||
|
||
To be timely and keep the number of notifications to a reasonable amount when multiple models fail, dbt observes the following guidelines when notifying the owners: |
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.
Change "multiple models fail" to "when multiple models or tests trigger notifications" because it might not just be models failing (could be a whole lot that warn)
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.
lgtm!
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.
Hey @nghi-ly just curious if we can elaborate on how to set up tests and had two very minor suggs.
Co-authored-by: Leona B. Campbell <[email protected]>
Co-authored-by: Leona B. Campbell <[email protected]>
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.
🚢 it!
What are you changing in this pull request and why?
Beta docs for model-level notifications
Checklist
ADDING PAGE (if so, uncomment):
website/sidebars.js
🚀 Deployment available! Here are the direct links to the updated files: