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

[Improvement][Alert] Add a test send feature when creating an alert instance #15163

Merged
merged 17 commits into from
Nov 20, 2023
Merged

[Improvement][Alert] Add a test send feature when creating an alert instance #15163

merged 17 commits into from
Nov 20, 2023

Conversation

qingwli
Copy link
Member

@qingwli qingwli commented Nov 15, 2023

@qingwli qingwli self-assigned this Nov 15, 2023
@qingwli qingwli added the improvement make more easy to user or prompt friendly label Nov 15, 2023
@qingwli qingwli added this to the 3.3.0 milestone Nov 15, 2023
@github-actions github-actions bot added UI ui and front end related backend document labels Nov 15, 2023
@codecov-commenter
Copy link

codecov-commenter commented Nov 15, 2023 •
edited
Loading

Codecov Report

Attention: 30 lines in your changes are missing coverage. Please review.

Comparison is base (2f28257) 38.24% compared to head (01eca71) 38.28%.

❗ Current head 01eca71 differs from pull request most recent head 660680e. Consider uploading reports for the commit 660680e to get more accurate results

Files Patch % Lines
...scheduler/alert/service/AlertBootstrapService.java 57.50% 15 Missing and 2 partials ⚠️
...i/service/impl/AlertPluginInstanceServiceImpl.java 73.07% 7 Missing ⚠️
.../dolphinscheduler/alert/rpc/AlertOperatorImpl.java 0.00% 6 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##                dev   #15163      +/-   ##
============================================
+ Coverage     38.24%   38.28%   +0.03%     
- Complexity     4696     4701       +5     
============================================
  Files          1285     1285              
  Lines         45471    45547      +76     
  Branches       4952     4957       +5     
============================================
+ Hits          17391    17436      +45     
- Misses        26188    26217      +29     
- Partials       1892     1894       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ruanwenjun
ruanwenjun requested changes Nov 15, 2023
View reviewed changes
Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

Good idea, leave some comments.

...lphinscheduler-alert-server/src/main/java/org/apache/dolphinscheduler/alert/AlertServer.java Outdated Show resolved Hide resolved
...server/src/main/java/org/apache/dolphinscheduler/alert/service/ListenerEventPostService.java Outdated Show resolved Hide resolved
dolphinscheduler-api/pom.xml Outdated Show resolved Hide resolved
@ruanwenjun ruanwenjun added the feature new feature label Nov 15, 2023
songjianet
songjianet requested changes Nov 16, 2023
View reviewed changes
dolphinscheduler-ui/src/views/security/alarm-instance-manage/use-detail.ts Outdated
: `${t('security.alarm_instance.test_send')} ${t('home.success')}`
)
status.testing = false
} catch (err) {
Copy link
Member

Choose a reason for hiding this comment

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

please delete err, because it is not used.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

ruanwenjun
ruanwenjun previously approved these changes Nov 17, 2023
View reviewed changes
Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

Backend LGTM

@qingwli
Copy link
Member Author

qingwli commented Nov 17, 2023

@songjianet PTAL

Radeity
Radeity reviewed Nov 17, 2023
View reviewed changes
Copy link
Member

@Radeity Radeity left a comment

Choose a reason for hiding this comment

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

Others LGTM

docs/docs/en/guide/alert/alert_plugin_user_guide.md Outdated Show resolved Hide resolved
@ruanwenjun ruanwenjun force-pushed the alert-add-test-feature branch from 7821d01 to 7f69c4b Compare November 18, 2023 09:12
@qingwli @Radeity
Update docs/docs/en/guide/alert/alert_plugin_user_guide.md
6282711 
Co-authored-by: Aaron Wang <wangweirao16@gmail.com>
@sonarqubecloud SonarQubeCloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 9 Code Smells

63.2% 63.2% Coverage
0.0% 0.0% Duplication

@qingwli qingwli merged commit 6aa6e11 into apache:dev Nov 20, 2023
55 of 56 checks passed
@qingwli qingwli deleted the alert-add-test-feature branch November 20, 2023 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Radeity Radeity Radeity left review comments

@songjianet songjianet songjianet approved these changes

@ruanwenjun ruanwenjun ruanwenjun left review comments

@zhongjiajie zhongjiajie Awaiting requested review from zhongjiajie

@EricGao888 EricGao888 Awaiting requested review from EricGao888 EricGao888 is a code owner

@Amy0104 Amy0104 Awaiting requested review from Amy0104

@caishunfeng caishunfeng Awaiting requested review from caishunfeng caishunfeng is a code owner

@SbloodyS SbloodyS Awaiting requested review from SbloodyS SbloodyS is a code owner

Assignees

@qingwli qingwli

Labels
backend document feature new feature improvement make more easy to user or prompt friendly ready-to-merge UI ui and front end related
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.

[Improvement][Alert] Add a test send feature when creating an alert instance
6 participants
@qingwli @codecov-commenter @songjianet @ruanwenjun @Radeity @yeahhhz