feat: add test email functionality to SQL-based email alerts (#10476)

* added test email functionality

* formatting changes

* more formatting

* applied requested changes

* mypy

Co-authored-by: Jason Davis <@dropbox.com>
This commit is contained in:
Jason Davis 2020-07-30 13:27:22 -07:00 committed by GitHub
parent c9cb723cab
commit 7a329c25e9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 101 additions and 4 deletions

View File

@ -540,6 +540,7 @@ def schedule_alert_query( # pylint: disable=unused-argument
report_type: ScheduleType, report_type: ScheduleType,
schedule_id: int, schedule_id: int,
recipients: Optional[str] = None, recipients: Optional[str] = None,
is_test_alert: Optional[bool] = False,
) -> None: ) -> None:
model_cls = get_scheduler_model(report_type) model_cls = get_scheduler_model(report_type)
dbsession = db.create_scoped_session() dbsession = db.create_scoped_session()
@ -551,6 +552,10 @@ def schedule_alert_query( # pylint: disable=unused-argument
return return
if report_type == ScheduleType.alert: if report_type == ScheduleType.alert:
if is_test_alert and recipients:
deliver_alert(schedule, recipients)
return
if run_alert_query(schedule, dbsession): if run_alert_query(schedule, dbsession):
# deliver_dashboard OR deliver_slice # deliver_dashboard OR deliver_slice
return return
@ -564,10 +569,12 @@ class AlertState:
PASS = "pass" PASS = "pass"
def deliver_alert(alert: Alert) -> None: def deliver_alert(alert: Alert, recipients: Optional[str] = None) -> None:
logging.info("Triggering alert: %s", alert) logging.info("Triggering alert: %s", alert)
img_data = None img_data = None
images = {} images = {}
recipients = recipients or alert.recipients
if alert.slice: if alert.slice:
chart_url = get_url_path( chart_url = get_url_path(
@ -604,7 +611,7 @@ def deliver_alert(alert: Alert) -> None:
image_url=image_url, image_url=image_url,
) )
_deliver_email(alert.recipients, deliver_as_group, subject, body, data, images) _deliver_email(recipients, deliver_as_group, subject, body, data, images)
def run_alert_query(alert: Alert, dbsession: Session) -> Optional[bool]: def run_alert_query(alert: Alert, dbsession: Session) -> Optional[bool]:

View File

@ -832,6 +832,13 @@ def get_email_address_list(address_string: str) -> List[str]:
return [x.strip() for x in address_string_list if x.strip()] return [x.strip() for x in address_string_list if x.strip()]
def get_email_address_str(address_string: str) -> str:
address_list = get_email_address_list(address_string)
address_list_str = ", ".join(address_list)
return address_list_str
def choicify(values: Iterable[Any]) -> List[Tuple[Any, Any]]: def choicify(values: Iterable[Any]) -> List[Tuple[Any, Any]]:
"""Takes an iterable and makes an iterable of tuples with it""" """Takes an iterable and makes an iterable of tuples with it"""
return [(v, v) for v in values] return [(v, v) for v in values]

View File

@ -14,14 +14,21 @@
# 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.
from typing import Dict, Optional, Union
from croniter import croniter
from flask_appbuilder import CompactCRUDMixin from flask_appbuilder import CompactCRUDMixin
from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_appbuilder.models.sqla.interface import SQLAInterface
from flask_babel import lazy_gettext as _ from flask_babel import lazy_gettext as _
from wtforms import BooleanField, Form, StringField
from superset.constants import RouteMethod from superset.constants import RouteMethod
from superset.models.alerts import Alert, AlertLog from superset.models.alerts import Alert, AlertLog
from superset.utils.core import markdown from superset.models.schedules import ScheduleType
from superset.tasks.schedules import schedule_alert_query
from superset.utils.core import get_email_address_str, markdown
from ..exceptions import SupersetException
from .base import SupersetModelView from .base import SupersetModelView
# TODO: access control rules for this module # TODO: access control rules for this module
@ -44,6 +51,10 @@ class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors
datamodel = SQLAInterface(Alert) datamodel = SQLAInterface(Alert)
route_base = "/alert" route_base = "/alert"
include_route_methods = RouteMethod.CRUD_SET include_route_methods = RouteMethod.CRUD_SET
_extra_data: Dict[str, Union[bool, Optional[str]]] = {
"test_alert": False,
"test_email_recipients": None,
}
list_columns = ( list_columns = (
"label", "label",
@ -68,6 +79,8 @@ class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors
# "dashboard", # "dashboard",
"log_retention", "log_retention",
"grace_period", "grace_period",
"test_alert",
"test_email_recipients",
) )
label_columns = { label_columns = {
"sql": "SQL", "sql": "SQL",
@ -95,5 +108,46 @@ class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors
"Superset nags you again." "Superset nags you again."
), ),
} }
add_form_extra_fields = {
"test_alert": BooleanField(
"Send Test Alert",
default=False,
description="If enabled, a test alert will be sent on the creation / update"
" of an active alert. All alerts after will be sent only if the SQL "
"statement defined above returns True.",
),
"test_email_recipients": StringField(
"Test Email Recipients",
default=None,
description="List of recipients to send test email to. "
"If empty, an email will be sent to the original recipients.",
),
}
edit_form_extra_fields = add_form_extra_fields
edit_columns = add_columns edit_columns = add_columns
related_views = [AlertLogModelView] related_views = [AlertLogModelView]
def process_form(self, form: Form, is_created: bool) -> None:
email_recipients = None
if form.test_email_recipients.data:
email_recipients = get_email_address_str(form.test_email_recipients.data)
self._extra_data["test_alert"] = form.test_alert.data
self._extra_data["test_email_recipients"] = email_recipients
def pre_add(self, item: "AlertModelView") -> None:
item.recipients = get_email_address_str(item.recipients)
if not croniter.is_valid(item.crontab):
raise SupersetException("Invalid crontab format")
def post_add(self, item: "AlertModelView") -> None:
if self._extra_data["test_alert"]:
recipients = self._extra_data["test_email_recipients"] or item.recipients
args = (ScheduleType.alert, item.id)
kwargs = dict(recipients=recipients, is_test_alert=True)
schedule_alert_query.apply_async(args=args, kwargs=kwargs)
def post_update(self, item: "AlertModelView") -> None:
self.post_add(item)

View File

@ -22,8 +22,9 @@ import pytest
from superset import db from superset import db
from superset.models.alerts import Alert, AlertLog from superset.models.alerts import Alert, AlertLog
from superset.models.schedules import ScheduleType
from superset.models.slice import Slice from superset.models.slice import Slice
from superset.tasks.schedules import run_alert_query from superset.tasks.schedules import run_alert_query, schedule_alert_query
from superset.utils import core as utils from superset.utils import core as utils
from tests.test_app import app from tests.test_app import app
@ -112,3 +113,31 @@ def test_run_alert_query(mock_error, mock_deliver, setup_database):
run_alert_query(database.query(Alert).filter_by(id=5).one(), database) run_alert_query(database.query(Alert).filter_by(id=5).one(), database)
assert mock_deliver.call_count == 1 assert mock_deliver.call_count == 1
assert mock_error.call_count == 4 assert mock_error.call_count == 4
@patch("superset.tasks.schedules.deliver_alert")
@patch("superset.tasks.schedules.run_alert_query")
def test_schedule_alert_query(mock_run_alert, mock_deliver_alert, setup_database):
database = setup_database
active_alert = database.query(Alert).filter_by(id=1).one()
inactive_alert = database.query(Alert).filter_by(id=3).one()
# Test that inactive alerts are no processed
schedule_alert_query(report_type=ScheduleType.alert, schedule_id=inactive_alert.id)
assert mock_run_alert.call_count == 0
assert mock_deliver_alert.call_count == 0
# Test that active alerts with no recipients passed in are processed regularly
schedule_alert_query(report_type=ScheduleType.alert, schedule_id=active_alert.id)
assert mock_run_alert.call_count == 1
assert mock_deliver_alert.call_count == 0
# Test that active alerts sent as a test are delivered immediately
schedule_alert_query(
report_type=ScheduleType.alert,
schedule_id=active_alert.id,
recipients="testing@email.com",
is_test_alert=True,
)
assert mock_run_alert.call_count == 1
assert mock_deliver_alert.call_count == 1