Skip to content

Conversation

@toujours33
Copy link
Contributor

@toujours33 toujours33 commented Nov 20, 2024

Purpose of the pull request

This pull request fix alert send status. fix #16820

Brief change log

  • Change failureCount and successCount for alertSendStatus
  • Add UT to validate alertStatus.

Verify this pull request

This change added tests and can be verified as follows:

  • Expand AlertSenderTest.testRun to verify the change

@toujours33 toujours33 requested a review from SbloodyS as a code owner November 20, 2024 03:38
@boring-cyborg
Copy link

boring-cyborg bot commented Nov 20, 2024

Thanks for opening this pull request! Please check out our contributing guidelines. (https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md)

@SbloodyS SbloodyS added this to the 3.3.0 milestone Nov 20, 2024
@SbloodyS SbloodyS added bug Something isn't working first time contributor First-time contributor labels Nov 20, 2024
@SbloodyS SbloodyS requested a review from ruanwenjun November 20, 2024 03:40
@toujours33 toujours33 changed the title [FIX-16820][Alert] fix alert status after alert sent by AlertSender [Fix-16820][Alert] fix alert status after alert sent by AlertSender Nov 20, 2024
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.

LGTM

Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

+1

@sonarqubecloud
Copy link

@SbloodyS SbloodyS merged commit 76a962f into apache:dev Nov 20, 2024
65 of 66 checks passed
@boring-cyborg
Copy link

boring-cyborg bot commented Nov 20, 2024

Awesome work, congrats on your first merged pull request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend bug Something isn't working first time contributor First-time contributor test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [Alert] AlertSendStatus doesn't make any sense.

3 participants