bkyryliuk commented on a change in pull request #10476:
URL:
https://github.com/apache/incubator-superset/pull/10476#discussion_r462564227
##########
File path: superset/tasks/schedules.py
##########
@@ -551,7 +551,12 @@ def schedule_alert_query( # pylint:
disable=unused-argument
return
if report_type == ScheduleType.alert:
- if run_alert_query(schedule, dbsession):
+ # recipients is only specified when task is called from view for a
test email
Review comment:
add additional flag to determine if that is a test run e.g. you already
have a flag in the form
##########
File path: superset/views/alerts.py
##########
@@ -95,5 +101,40 @@ class AlertModelView(SupersetModelView): # pylint:
disable=too-many-ancestors
"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 active alerts.",
Review comment:
explain that email will be send only if alert will fail
##########
File path: superset/views/alerts.py
##########
@@ -95,5 +101,40 @@ class AlertModelView(SupersetModelView): # pylint:
disable=too-many-ancestors
"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 active alerts.",
+ ),
+ "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
related_views = [AlertLogModelView]
+
+ def process_form(self, form: Form, is_created: bool) -> None:
+ if form.test_email_recipients.data:
+ test_email_recipients = form.test_email_recipients.data.strip()
+ else:
+ test_email_recipients = None
+
+ self._extra_data["test_alert"] = form.test_alert.data
Review comment:
https://github.com/apache/incubator-superset/blob/master/superset/views/schedules.py#L120
could be needed as well, e.g. normalizing the email list
##########
File path: tests/alerts_tests.py
##########
@@ -112,3 +113,30 @@ def test_run_alert_query(mock_error, mock_deliver,
setup_database):
run_alert_query(database.query(Alert).filter_by(id=5).one(), database)
assert mock_deliver.call_count == 1
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):
Review comment:
it would be nice to add the end to end test as well, it can be done in
the separate PR.
Example:
https://github.com/apache/incubator-superset/blob/master/tests/core_tests.py#L827
##########
File path: superset/views/alerts.py
##########
@@ -95,5 +101,40 @@ class AlertModelView(SupersetModelView): # pylint:
disable=too-many-ancestors
"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 active alerts.",
+ ),
+ "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
related_views = [AlertLogModelView]
+
+ def process_form(self, form: Form, is_created: bool) -> None:
+ if form.test_email_recipients.data:
Review comment:
do this instead
```
test_email_recipients = None
if form.test_email_recipients.data:
test_email_recipients = form.test_email_recipients.data.strip()
```
##########
File path: superset/views/alerts.py
##########
@@ -44,6 +50,7 @@ class AlertModelView(SupersetModelView): # pylint:
disable=too-many-ancestors
datamodel = SQLAInterface(Alert)
route_base = "/alert"
include_route_methods = RouteMethod.CRUD_SET
+ _extra_data = {"test_alert": False, "test_email_recipients": None}
Review comment:
add typing annotation here
##########
File path: superset/views/alerts.py
##########
@@ -95,5 +104,46 @@ class AlertModelView(SupersetModelView): # pylint:
disable=too-many-ancestors
"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
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 # type:
ignore
Review comment:
resolve type: ignore - you can cast
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
