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

Revive PR: Adds mechanism to prevent spam from unhealthy notifications #1877

Conversation

cieciurm
Copy link
Contributor

@cieciurm cieciurm commented Jul 7, 2023

What this PR does / why we need it:

This PR revives #592
Original description:

Adds mechanism to prevent spam from unhealthy notifications.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

  • Merged with master
  • Tests fixed
  • Formatting fixed
  • Public API approved

Does this PR introduce a user-facing change?:

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Unit tests passing
  • End-to-end tests passing
  • Extended the documentation
  • Provided sample for the feature

@github-actions github-actions bot added UI documentation An issue or pull request regarding documentation improvements labels Jul 7, 2023
@cieciurm cieciurm force-pushed the feature/configure-unhealth-notification-to-avoid-spam branch from 6213275 to 99760d8 Compare July 7, 2023 06:04
@cieciurm cieciurm force-pushed the feature/configure-unhealth-notification-to-avoid-spam branch 2 times, most recently from 70104b9 to 640b3e3 Compare July 7, 2023 06:20
@cieciurm cieciurm force-pushed the feature/configure-unhealth-notification-to-avoid-spam branch from 640b3e3 to d3b6df7 Compare July 7, 2023 06:22
@cieciurm
Copy link
Contributor Author

cieciurm commented Jul 7, 2023

The CI seems to be flaky - should_return_too_many_requests_status_using_default_server_max_active_requests suddenly failed, any chance you could re-run the CI? cc @sungam3r

@cieciurm
Copy link
Contributor Author

cieciurm commented Jul 7, 2023

PR adjusted
cc @sungam3r

EDIT: The flaky test should_return_too_many_requests_status_using_default_server_max_active_requests failed yet again. Could you re-run the workflow?

README.md Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Merging #1877 (7a0ff36) into master (a65b73c) will decrease coverage by 0.19%.
The diff coverage is 20.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master    #1877      +/-   ##
==========================================
- Coverage   66.94%   66.76%   -0.19%     
==========================================
  Files         228      228              
  Lines        8135     8163      +28     
  Branches      573      577       +4     
==========================================
+ Hits         5446     5450       +4     
- Misses       2545     2567      +22     
- Partials      144      146       +2     
Flag Coverage Δ
UI 65.73% <20.00%> (-0.35%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/HealthChecks.UI/Configuration/Settings.cs 82.82% <20.00%> (-3.35%) ⬇️
...I/Core/HostedService/HealthCheckReportCollector.cs 60.56% <20.00%> (-5.23%) ⬇️

... and 1 file with indirect coverage changes

README.md Outdated Show resolved Hide resolved
cieciurm and others added 3 commits July 7, 2023 15:11
Co-authored-by: Ivan Maximov <sungam3r@yandex.ru>
Co-authored-by: Ivan Maximov <sungam3r@yandex.ru>
Co-authored-by: Ivan Maximov <sungam3r@yandex.ru>
@cieciurm
Copy link
Contributor Author

cieciurm commented Jul 7, 2023

@sungam3r - you suggestions are applied now :)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@cieciurm
Copy link
Contributor Author

cieciurm commented Jul 9, 2023

Public API has been sorted alphabetically and workflow succeeded 🎉

@sungam3r sungam3r merged commit f7f8cf2 into Xabaril:master Jul 9, 2023
@cieciurm cieciurm deleted the feature/configure-unhealth-notification-to-avoid-spam branch July 10, 2023 04:37
@sungam3r sungam3r mentioned this pull request Jul 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation An issue or pull request regarding documentation improvements UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants