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

Model-level notifications #6218

Merged
merged 28 commits into from
Oct 17, 2024
Merged

Model-level notifications #6218

merged 28 commits into from
Oct 17, 2024

Conversation

nghi-ly
Copy link
Contributor

@nghi-ly nghi-ly commented Oct 2, 2024

What are you changing in this pull request and why?

Beta docs for model-level notifications

  • New page
  • Update sidebar so new page is below Job notifications page
  • Add feature card to two landing pages

Checklist

ADDING PAGE (if so, uncomment):

  • Add/remove page in website/sidebars.js
  • Provide a unique filename for new pages

🚀 Deployment available! Here are the direct links to the updated files:

@nghi-ly nghi-ly requested a review from a team as a code owner October 2, 2024 19:21
Copy link

vercel bot commented Oct 2, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
docs-getdbt-com ✅ Ready (Inspect) Visit Preview Oct 17, 2024 6:39pm

@github-actions github-actions bot added content Improvements or additions to content Docs team Authored by the Docs team @dbt Labs size: medium This change will take up to a week to address labels Oct 2, 2024
@nghi-ly nghi-ly requested a review from reubenmc October 2, 2024 19:28
Copy link

@reubenmc reubenmc left a 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

website/docs/docs/deploy/model-notifications.md Outdated Show resolved Hide resolved
website/docs/docs/deploy/model-notifications.md Outdated Show resolved Hide resolved
website/docs/docs/deploy/model-notifications.md Outdated Show resolved Hide resolved
@rviswanathan-dbt
Copy link

At a high level, depending on the UI settings, users can choose to be notified on -
For models: fail/success
For tests on models: fail/success/warn

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.

Copy link

@reubenmc reubenmc left a 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:

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)

Copy link

@rviswanathan-dbt rviswanathan-dbt left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Collaborator

@runleonarun runleonarun left a 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.

website/docs/docs/deploy/model-notifications.md Outdated Show resolved Hide resolved
website/docs/docs/deploy/model-notifications.md Outdated Show resolved Hide resolved
website/docs/docs/deploy/model-notifications.md Outdated Show resolved Hide resolved
nghi-ly and others added 2 commits October 17, 2024 08:26
Co-authored-by: Leona B. Campbell <3880403+runleonarun@users.noreply.github.com>
Co-authored-by: Leona B. Campbell <3880403+runleonarun@users.noreply.github.com>
Copy link
Collaborator

@runleonarun runleonarun left a comment

Choose a reason for hiding this comment

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

🚢 it!

@nghi-ly nghi-ly merged commit 26b663a into current Oct 17, 2024
9 checks passed
@nghi-ly nghi-ly deleted the ly-docs-beta-model-notifications branch October 17, 2024 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto update content Improvements or additions to content Docs team Authored by the Docs team @dbt Labs size: medium This change will take up to a week to address
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants