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.

Sign up for GitHub

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

Jump to bottom

Improve Slack notification message and fix leakages #825

Merged
merged 9 commits into from
Jul 21, 2022
Merged

Improve Slack notification message and fix leakages #825

merged 9 commits into from
Jul 21, 2022

Conversation

DementevNikita
Copy link
Member

What changes did you make? (Give an overview)
This PR contain improvements for Slack/Webhook notifications. Several bugs and leakages were fixed. Slack message formatting has been improved.

How to test
The best way would be:

  1. Create Slack workspace and create an incoming webhook.
  2. Configure the platform to send notifications via Slack
  3. Inject complex data graph and emulate several Alert types using Ingestion API (remove an existing column, inject failed job run/test run)
  4. Assert that Slack messages appear and they are providing correct data

closes #742

@DementevNikita DementevNikita added kind: bug Something isn't working kind: enhancement Enhancement of existing feature scope: backend Backend task labels Jul 19, 2022
@DementevNikita DementevNikita added this to the 0.6 milestone Jul 19, 2022
@DementevNikita DementevNikita self-assigned this Jul 19, 2022
@DementevNikita DementevNikita requested review from a team as code owners July 19, 2022 10:06
@github-actions

This comment has been minimized.

Sign in to view
damirabdul
damirabdul reviewed Jul 19, 2022
View reviewed changes
Copy link
Contributor

@damirabdul damirabdul left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions
Copy link

Test Results

  33 files    33 suites   1m 11s ⏱️
182 tests 182 ✔️ 0 💤 0 ❌
205 runs  205 ✔️ 0 💤 0 ❌

Results for commit 0c843eb.

germanosin
germanosin approved these changes Jul 20, 2022
View reviewed changes
Copy link
Member

@germanosin germanosin left a comment

Choose a reason for hiding this comment

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

LGTM

@DementevNikita DementevNikita merged commit 021a6d0 into main Jul 21, 2022
@DementevNikita DementevNikita deleted the enhancement/alert_formatting branch July 21, 2022 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@damirabdul damirabdul damirabdul approved these changes

@germanosin germanosin germanosin approved these changes

Assignees

@DementevNikita DementevNikita

Labels
kind: bug Something isn't working kind: enhancement Enhancement of existing feature scope: backend Backend task
Projects
None yet
Milestone
0.6
Development

Successfully merging this pull request may close these issues.

Capture and send Alert data to incoming webhooks
3 participants
@DementevNikita @germanosin @damirabdul