Alerting integration #542
Alerting integration #542
Conversation
Created notification content class
Feature/issue notification
Feature/issue by group
# Conflicts: # flask_monitoringdashboard/core/exceptions/exception_collector.py # flask_monitoringdashboard/database/exception_occurrence.py
…-issues Feature/config for GitHub issues
# Conflicts: # flask_monitoringdashboard/core/exceptions/exception_collector.py
Email notification, modifiable notifcation type
Added chat integration
Master pull
…eature/chat_integration
Feature/chat integration
mircealungu
left a comment
mircealungu
left a comment
There was a problem hiding this comment.
amazing work folks.
a few small points and I look forward to deploying this.
|
|
||
| [alerting] | ||
| ENABLED=False | ||
| TYPE=email |
There was a problem hiding this comment.
Do we really need the TYPE? We can simply test in the code for the existence of the ENVVARS? Or is there a case where we want to have the TYPE defined independently from the enviers?
|
|
||
| At the moment the secrets and variables for the issue are saved in a .env file | ||
|
|
||
| There is a template with file name `.env_template` that can be used to fill in the info needed for the issue creation alert to work. |
There was a problem hiding this comment.
I don't see such a file. And also, I see that the file above -- config.cfg -- already definies secrets and variables. So is this still needed?
There was a problem hiding this comment.
No it is not needed and should be deleted. Was a part of previous setup. Now the config.cfg is used
| template = template_env.get_template('report.html') | ||
|
|
||
|
|
||
| class AlertContent: |
There was a problem hiding this comment.
A small comment explaining the use of this class would be good
There was a problem hiding this comment.
I don't know why but alert. Seems like a strange name for this module. Either alerting or alerts would somehow be more understandable for me.
|
|
||
| def create_teams_payload(alert_content: AlertContent): | ||
| return { | ||
| "type": "message", |
There was a problem hiding this comment.
haha. Why am I not surprised that the Microsoft alternative is the most complicated one?
| @@ -0,0 +1,16 @@ | |||
| class GitHubRequestInfo: | |||
There was a problem hiding this comment.
Do you really need this class?
There was a problem hiding this comment.
I guess we could do it without. Is that preferred?
| e_copy = _get_copy_of_exception(e) | ||
|
|
||
| if config.alert_enabled: | ||
| _notify(e_copy, session, config) |
There was a problem hiding this comment.
Why do I need a copy? Maybe at the comment too? It's not evident
There was a problem hiding this comment.
I'm not sure, we will look into it.
| return new_exc | ||
|
|
||
|
|
||
| def _notify( |
There was a problem hiding this comment.
Maybe move to the alerts package?
| templateUrl: 'static/pages/database_management.html', | ||
| controller: ['$scope', '$http', 'menuService', 'endpointService', 'modalService', DatabaseManagementController] | ||
| }) | ||
| .when('/alerting_settings', { |
There was a problem hiding this comment.
oh. we have also a UI for the alerting settings :) COOL!
| :return: A JSON-object with alert configuration details | ||
| """ | ||
| # TODO | ||
| # post_to_back_if_telemetry_enabled(**{'name': 'deploy_alert_config'}) |
There was a problem hiding this comment.
what is this for? not for sending telemetry info about the installation is it? because then email is too private to collect it.
4 participants
No description provided.