Internal monitor notifications set through config#96
Merged
GabrielSalla merged 8 commits intomainfrom Jun 19, 2025
Merged
Conversation
43910c9 to
b23fb65
Compare
b23fb65 to
dacf67d
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR integrates internal monitor notifications into the application by refactoring notification creation and updating configuration and documentation accordingly.
- Updates and extends tests to cover SlackNotification creation and internal monitor notification behavior.
- Refactors the SlackNotification and BaseNotification to use AlertPriority instead of int, and adjusts config and docs to match.
- Updates plugin initialization order in main and cleans up module exports.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/plugins/slack/notifications/test_slack_notification.py | Adds parameterized tests for SlackNotification creation. |
| tests/notifications/test_internal_monitor_notification.py | Introduces tests for internal monitor notifications and error cases. |
| tests/module_loader/test_checker.py | Updates BaseNotification type hints to use AlertPriority. |
| src/plugins/slack/notifications/slack_notification.py | Refactors SlackNotification creation logic with improved types. |
| src/plugins/slack/init.py | Adjusts exports to include notifications module. |
| src/notifications/internal_monitor_notification.py | Implements internal monitor notification creation logic. |
| src/notifications/base_notification.py | Updates BaseNotification protocol for AlertPriority usage. |
| src/main.py | Reorders plugin initialization for dependency correctness. |
| src/configs/configs_loader.py | Adds internal_monitors_notification config loading. |
| sample_monitors/test_monitor/test_monitor.py | Switches sample monitor notification to use internal notification. |
| resources/kubernetes_template/config_map.yaml | Adds internal_monitors_notification configuration. |
| internal_monitors/monitor_high_active_issues_count/monitor_high_active_issues_count.py | Updates notification instantiation for internal monitors. |
| docs/plugins/slack.md | Revises documentation to clarify usage of environment variables. |
| docs/plugins/plugins.md | Updates examples to reflect AlertPriority usage. |
| docs/configuration_file.md | Expands configuration documentation for internal monitors. |
| configs/configs.yaml | Adds internal_monitors_notification config section. |
| configs/configs-scalable.yaml | Adds internal_monitors_notification config section. |
Comments suppressed due to low confidence (3)
tests/plugins/slack/notifications/test_slack_notification.py:29
- [nitpick] Consider renaming the test function 'test_slacknotification_create' to 'test_slack_notification_create' for improved readability and consistency in naming conventions.
@pytest.mark.parametrize(
docs/plugins/slack.md:15
- The documentation now includes redundant entries for SLACK_MAIN_CHANNEL and SLACK_MAIN_MENTION; consider consolidating these descriptions for clarity and consistency.
- `SLACK_MAIN_CHANNEL`: The Slack channel where notifications for **internal monitors** and **sample monitors** will be sent. Example: `C0011223344`.
internal_monitors/monitor_high_active_issues_count/monitor_high_active_issues_count.py:13
- The 'PriorityLevels' import appears to be unused; consider removing it to keep the code clean and avoid unnecessary dependencies.
PriorityLevels,
dacf67d to
a1a425b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.