mirror of https://github.com/apache/superset.git
feat: only send alert error emails to owners of the alert (#13862)
* only send alert error emails to owners of the alert * reformat long lines * fix send to owners and add tests * fix pylint errors * fix formatting
This commit is contained in:
parent
778bb8e540
commit
911462a148
|
@ -14,6 +14,7 @@
|
||||||
# KIND, either express or implied. See the License for the
|
# KIND, either express or implied. See the License for the
|
||||||
# specific language governing permissions and limitations
|
# specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
import json
|
||||||
import logging
|
import logging
|
||||||
from datetime import datetime, timedelta
|
from datetime import datetime, timedelta
|
||||||
from typing import Any, List, Optional
|
from typing import Any, List, Optional
|
||||||
|
@ -29,6 +30,8 @@ from superset.commands.exceptions import CommandException
|
||||||
from superset.extensions import feature_flag_manager
|
from superset.extensions import feature_flag_manager
|
||||||
from superset.models.reports import (
|
from superset.models.reports import (
|
||||||
ReportExecutionLog,
|
ReportExecutionLog,
|
||||||
|
ReportRecipients,
|
||||||
|
ReportRecipientType,
|
||||||
ReportSchedule,
|
ReportSchedule,
|
||||||
ReportScheduleType,
|
ReportScheduleType,
|
||||||
ReportState,
|
ReportState,
|
||||||
|
@ -226,14 +229,17 @@ class BaseReportState:
|
||||||
description=self._report_schedule.description,
|
description=self._report_schedule.description,
|
||||||
)
|
)
|
||||||
|
|
||||||
def _send(self, notification_content: NotificationContent) -> None:
|
@staticmethod
|
||||||
|
def _send(
|
||||||
|
notification_content: NotificationContent, recipients: List[ReportRecipients]
|
||||||
|
) -> None:
|
||||||
"""
|
"""
|
||||||
Sends a notification to all recipients
|
Sends a notification to all recipients
|
||||||
|
|
||||||
:raises: ReportScheduleNotificationError
|
:raises: ReportScheduleNotificationError
|
||||||
"""
|
"""
|
||||||
notification_errors = []
|
notification_errors = []
|
||||||
for recipient in self._report_schedule.recipients:
|
for recipient in recipients:
|
||||||
notification = create_notification(recipient, notification_content)
|
notification = create_notification(recipient, notification_content)
|
||||||
try:
|
try:
|
||||||
notification.send()
|
notification.send()
|
||||||
|
@ -250,7 +256,7 @@ class BaseReportState:
|
||||||
:raises: ReportScheduleNotificationError
|
:raises: ReportScheduleNotificationError
|
||||||
"""
|
"""
|
||||||
notification_content = self._get_notification_content()
|
notification_content = self._get_notification_content()
|
||||||
self._send(notification_content)
|
self._send(notification_content, self._report_schedule.recipients)
|
||||||
|
|
||||||
def send_error(self, name: str, message: str) -> None:
|
def send_error(self, name: str, message: str) -> None:
|
||||||
"""
|
"""
|
||||||
|
@ -259,7 +265,17 @@ class BaseReportState:
|
||||||
:raises: ReportScheduleNotificationError
|
:raises: ReportScheduleNotificationError
|
||||||
"""
|
"""
|
||||||
notification_content = NotificationContent(name=name, text=message)
|
notification_content = NotificationContent(name=name, text=message)
|
||||||
self._send(notification_content)
|
|
||||||
|
# filter recipients to recipients who are also owners
|
||||||
|
owner_recipients = [
|
||||||
|
ReportRecipients(
|
||||||
|
type=ReportRecipientType.EMAIL,
|
||||||
|
recipient_config_json=json.dumps({"target": owner.email}),
|
||||||
|
)
|
||||||
|
for owner in self._report_schedule.owners
|
||||||
|
]
|
||||||
|
|
||||||
|
self._send(notification_content, owner_recipients)
|
||||||
|
|
||||||
def is_in_grace_period(self) -> bool:
|
def is_in_grace_period(self) -> bool:
|
||||||
"""
|
"""
|
||||||
|
|
|
@ -26,7 +26,7 @@ from flask_sqlalchemy import BaseQuery
|
||||||
from freezegun import freeze_time
|
from freezegun import freeze_time
|
||||||
from sqlalchemy.sql import func
|
from sqlalchemy.sql import func
|
||||||
|
|
||||||
from superset import db
|
from superset import db, security_manager
|
||||||
from superset.models.core import Database
|
from superset.models.core import Database
|
||||||
from superset.models.dashboard import Dashboard
|
from superset.models.dashboard import Dashboard
|
||||||
from superset.models.reports import (
|
from superset.models.reports import (
|
||||||
|
@ -130,6 +130,12 @@ def create_report_notification(
|
||||||
report_type = report_type or ReportScheduleType.REPORT
|
report_type = report_type or ReportScheduleType.REPORT
|
||||||
target = email_target or slack_channel
|
target = email_target or slack_channel
|
||||||
config_json = {"target": target}
|
config_json = {"target": target}
|
||||||
|
owner = (
|
||||||
|
db.session.query(security_manager.user_model)
|
||||||
|
.filter_by(email="admin@fab.org")
|
||||||
|
.one_or_none()
|
||||||
|
)
|
||||||
|
|
||||||
if slack_channel:
|
if slack_channel:
|
||||||
recipient = ReportRecipients(
|
recipient = ReportRecipients(
|
||||||
type=ReportRecipientType.SLACK,
|
type=ReportRecipientType.SLACK,
|
||||||
|
@ -151,6 +157,7 @@ def create_report_notification(
|
||||||
dashboard=dashboard,
|
dashboard=dashboard,
|
||||||
database=database,
|
database=database,
|
||||||
recipients=[recipient],
|
recipients=[recipient],
|
||||||
|
owners=[owner],
|
||||||
validator_type=validator_type,
|
validator_type=validator_type,
|
||||||
validator_config_json=validator_config_json,
|
validator_config_json=validator_config_json,
|
||||||
grace_period=grace_period,
|
grace_period=grace_period,
|
||||||
|
@ -890,7 +897,7 @@ def test_soft_timeout_alert(email_mock, create_alert_email_chart):
|
||||||
|
|
||||||
notification_targets = get_target_from_report_schedule(create_alert_email_chart)
|
notification_targets = get_target_from_report_schedule(create_alert_email_chart)
|
||||||
# Assert the email smtp address, asserts a notification was sent with the error
|
# Assert the email smtp address, asserts a notification was sent with the error
|
||||||
assert email_mock.call_args[0][0] == notification_targets[0]
|
assert email_mock.call_args[0][0] == "admin@fab.org"
|
||||||
|
|
||||||
assert_log(
|
assert_log(
|
||||||
ReportState.ERROR, error_message="A timeout occurred while executing the query."
|
ReportState.ERROR, error_message="A timeout occurred while executing the query."
|
||||||
|
@ -919,9 +926,8 @@ def test_soft_timeout_screenshot(screenshot_mock, email_mock, create_alert_email
|
||||||
test_id, create_alert_email_chart.id, datetime.utcnow()
|
test_id, create_alert_email_chart.id, datetime.utcnow()
|
||||||
).run()
|
).run()
|
||||||
|
|
||||||
notification_targets = get_target_from_report_schedule(create_alert_email_chart)
|
|
||||||
# Assert the email smtp address, asserts a notification was sent with the error
|
# Assert the email smtp address, asserts a notification was sent with the error
|
||||||
assert email_mock.call_args[0][0] == notification_targets[0]
|
assert email_mock.call_args[0][0] == "admin@fab.org"
|
||||||
|
|
||||||
assert_log(
|
assert_log(
|
||||||
ReportState.ERROR, error_message="A timeout occurred while taking a screenshot."
|
ReportState.ERROR, error_message="A timeout occurred while taking a screenshot."
|
||||||
|
@ -948,7 +954,7 @@ def test_fail_screenshot(screenshot_mock, email_mock, create_report_email_chart)
|
||||||
|
|
||||||
notification_targets = get_target_from_report_schedule(create_report_email_chart)
|
notification_targets = get_target_from_report_schedule(create_report_email_chart)
|
||||||
# Assert the email smtp address, asserts a notification was sent with the error
|
# Assert the email smtp address, asserts a notification was sent with the error
|
||||||
assert email_mock.call_args[0][0] == notification_targets[0]
|
assert email_mock.call_args[0][0] == "admin@fab.org"
|
||||||
|
|
||||||
assert_log(
|
assert_log(
|
||||||
ReportState.ERROR, error_message="Failed taking a screenshot Unexpected error"
|
ReportState.ERROR, error_message="Failed taking a screenshot Unexpected error"
|
||||||
|
@ -997,7 +1003,7 @@ def test_invalid_sql_alert(email_mock, create_invalid_sql_alert_email_chart):
|
||||||
create_invalid_sql_alert_email_chart
|
create_invalid_sql_alert_email_chart
|
||||||
)
|
)
|
||||||
# Assert the email smtp address, asserts a notification was sent with the error
|
# Assert the email smtp address, asserts a notification was sent with the error
|
||||||
assert email_mock.call_args[0][0] == notification_targets[0]
|
assert email_mock.call_args[0][0] == "admin@fab.org"
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.usefixtures("create_invalid_sql_alert_email_chart")
|
@pytest.mark.usefixtures("create_invalid_sql_alert_email_chart")
|
||||||
|
@ -1018,7 +1024,7 @@ def test_grace_period_error(email_mock, create_invalid_sql_alert_email_chart):
|
||||||
create_invalid_sql_alert_email_chart
|
create_invalid_sql_alert_email_chart
|
||||||
)
|
)
|
||||||
# Assert the email smtp address, asserts a notification was sent with the error
|
# Assert the email smtp address, asserts a notification was sent with the error
|
||||||
assert email_mock.call_args[0][0] == notification_targets[0]
|
assert email_mock.call_args[0][0] == "admin@fab.org"
|
||||||
assert (
|
assert (
|
||||||
get_notification_error_sent_count(create_invalid_sql_alert_email_chart) == 1
|
get_notification_error_sent_count(create_invalid_sql_alert_email_chart) == 1
|
||||||
)
|
)
|
||||||
|
|
Loading…
Reference in New Issue