You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ma...@apache.org on 2020/10/02 16:57:37 UTC
[incubator-superset] 01/01: Revert "fix: superset alerting misc
fixes (#10891)"
This is an automated email from the ASF dual-hosted git repository.
maximebeauchemin pushed a commit to branch revert-10891-bogdan/alerting_fixes
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
commit 0df21d8c4d5d2d25823e47c4e7a88d34ed8b3023
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Fri Oct 2 09:57:15 2020 -0700
Revert "fix: superset alerting misc fixes (#10891)"
This reverts commit c3f1720456fd7e7d94b986b43b5b057e5236e65a.
---
superset/models/alerts.py | 6 +--
superset/tasks/alerts/validator.py | 14 ++++---
superset/tasks/schedules.py | 14 ++++---
superset/views/alerts.py | 77 +++++++++++++++++++++++++++++---------
tests/alerts_tests.py | 7 +---
5 files changed, 78 insertions(+), 40 deletions(-)
diff --git a/superset/models/alerts.py b/superset/models/alerts.py
index 065f7cf..95e7c06 100644
--- a/superset/models/alerts.py
+++ b/superset/models/alerts.py
@@ -59,7 +59,6 @@ class Alert(Model, AuditMixinNullable):
id = Column(Integer, primary_key=True)
label = Column(String(150), nullable=False)
active = Column(Boolean, default=True, index=True)
- # TODO(bkyryliuk): enforce minimal supported frequency
crontab = Column(String(50), nullable=False)
alert_type = Column(String(50))
@@ -67,7 +66,7 @@ class Alert(Model, AuditMixinNullable):
recipients = Column(Text)
slack_channel = Column(Text)
- # TODO(bkyryliuk): implement log_retention
+ # TODO: implement log_retention
log_retention = Column(Integer, default=90)
grace_period = Column(Integer, default=60 * 60 * 24)
@@ -204,8 +203,7 @@ class Validator(Model, AuditMixinNullable):
backref=backref("validators", cascade="all, delete-orphan"),
)
- @property
- def pretty_config(self) -> str:
+ def pretty_print(self) -> str:
""" String representing the comparison that will trigger a validator """
config = json.loads(self.config)
diff --git a/superset/tasks/alerts/validator.py b/superset/tasks/alerts/validator.py
index 68d9082..56dfad4 100644
--- a/superset/tasks/alerts/validator.py
+++ b/superset/tasks/alerts/validator.py
@@ -46,7 +46,7 @@ def check_validator(validator_type: str, config: str) -> None:
if validator_type == AlertValidatorType.operator.value:
- if not (config_dict.get("op") and config_dict.get("threshold") is not None):
+ if not (config_dict.get("op") and config_dict.get("threshold")):
raise SupersetException(
"Error: Operator Validator needs specified operator and threshold "
'values. Add "op" and "threshold" to config.'
@@ -87,13 +87,15 @@ def operator_validator(observer: SQLObserver, validator_config: str) -> bool:
Returns True if a SQLObserver's recent observation is greater than or equal to
the value given in the validator config
"""
+
observation = observer.get_last_observation()
- if not observation or observation.value in (None, np.nan):
- return False
+ if observation and observation.value not in (None, np.nan):
+ operator = json.loads(validator_config)["op"]
+ threshold = json.loads(validator_config)["threshold"]
+ if OPERATOR_FUNCTIONS[operator](observation.value, threshold):
+ return True
- operator = json.loads(validator_config)["op"]
- threshold = json.loads(validator_config)["threshold"]
- return OPERATOR_FUNCTIONS[operator](observation.value, threshold)
+ return False
def get_validator_function(
diff --git a/superset/tasks/schedules.py b/superset/tasks/schedules.py
index 39394c0..ea48543 100644
--- a/superset/tasks/schedules.py
+++ b/superset/tasks/schedules.py
@@ -591,7 +591,7 @@ def deliver_alert(
recipients = recipients or alert.recipients
slack_channel = slack_channel or alert.slack_channel
validation_error_message = (
- str(alert.observations[-1].value) + " " + alert.validators[0].pretty_config
+ str(alert.observations[-1].value) + " " + alert.validators[0].pretty_print()
if alert.validators
else ""
)
@@ -742,13 +742,15 @@ def validate_observations(alert_id: int, label: str, session: Session) -> bool:
"""
logger.info("Validating observations for alert <%s:%s>", alert_id, label)
+
alert = session.query(Alert).get(alert_id)
- if not alert.validators:
- return False
+ if alert.validators:
+ validator = alert.validators[0]
+ validate = get_validator_function(validator.validator_type)
+ if validate and validate(alert.sql_observer[0], validator.config):
+ return True
- validator = alert.validators[0]
- validate = get_validator_function(validator.validator_type)
- return bool(validate and validate(alert.sql_observer[0], validator.config))
+ return False
def next_schedules(
diff --git a/superset/views/alerts.py b/superset/views/alerts.py
index 5d0d1b8..c4de48b 100644
--- a/superset/views/alerts.py
+++ b/superset/views/alerts.py
@@ -14,10 +14,13 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+from typing import Dict, Optional, Union
+
from croniter import croniter
from flask_appbuilder import CompactCRUDMixin
from flask_appbuilder.models.sqla.interface import SQLAInterface
from flask_babel import lazy_gettext as _
+from wtforms import BooleanField, Form, StringField
from superset.constants import RouteMethod
from superset.models.alerts import (
@@ -27,7 +30,9 @@ from superset.models.alerts import (
SQLObserver,
Validator,
)
+from superset.models.schedules import ScheduleType
from superset.tasks.alerts.validator import check_validator
+from superset.tasks.schedules import schedule_alert_query
from superset.utils import core as utils
from superset.utils.core import get_email_address_str, markdown
@@ -42,7 +47,6 @@ class AlertLogModelView(
): # pylint: disable=too-many-ancestors
datamodel = SQLAInterface(AlertLog)
include_route_methods = {RouteMethod.LIST} | {"show"}
- base_order = ("dttm_start", "desc")
list_columns = (
"scheduled_dttm",
"dttm_start",
@@ -56,7 +60,6 @@ class AlertObservationModelView(
): # pylint: disable=too-many-ancestors
datamodel = SQLAInterface(SQLObservation)
include_route_methods = {RouteMethod.LIST} | {"show"}
- base_order = ("dttm", "desc")
list_title = _("List Observations")
show_title = _("Show Observation")
list_columns = (
@@ -127,9 +130,8 @@ class ValidatorInlineView( # pylint: disable=too-many-ancestors
add_columns = edit_columns
list_columns = [
- "alert.label",
"validator_type",
- "pretty_config",
+ "alert.label",
]
label_columns = {
@@ -178,6 +180,10 @@ class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors
datamodel = SQLAInterface(Alert)
route_base = "/alert"
include_route_methods = RouteMethod.CRUD_SET
+ _extra_data: Dict[str, Union[bool, Optional[str]]] = {
+ "test_alert": False,
+ "test_email_recipients": None,
+ }
list_columns = (
"label",
@@ -185,20 +191,7 @@ class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors
"last_eval_dttm",
"last_state",
"active",
- "owners",
- )
- show_columns = (
- "label",
- "active",
- "crontab",
- "owners",
- "slice",
- "recipients",
- "slack_channel",
- "log_retention",
- "grace_period",
- "last_eval_dttm",
- "last_state",
+ "creator",
)
order_columns = ["label", "last_eval_dttm", "last_state", "active"]
add_columns = (
@@ -215,6 +208,9 @@ class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors
# "dashboard",
"log_retention",
"grace_period",
+ "test_alert",
+ "test_email_recipients",
+ "test_slack_channel",
)
label_columns = {
"log_retention": _("Log Retentions (days)"),
@@ -234,6 +230,28 @@ class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors
),
}
+ 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.",
+ ),
+ "test_slack_channel": StringField(
+ "Test Slack Channel",
+ default=None,
+ description="A slack channel to send a test message to. "
+ "If empty, an alert will be sent to the original channel.",
+ ),
+ }
+ edit_form_extra_fields = add_form_extra_fields
edit_columns = add_columns
related_views = [
AlertObservationModelView,
@@ -242,11 +260,34 @@ class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors
SQLObserverInlineView,
]
+ 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)
+
+ test_slack_channel = (
+ form.test_slack_channel.data.strip()
+ if form.test_slack_channel.data
+ else None
+ )
+
+ self._extra_data["test_alert"] = form.test_alert.data
+ self._extra_data["test_email_recipients"] = email_recipients
+ self._extra_data["test_slack_channel"] = test_slack_channel
+
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
+ slack_channel = self._extra_data["test_slack_channel"] or item.slack_channel
+ args = (ScheduleType.alert, item.id)
+ kwargs = dict(recipients=recipients, slack_channel=slack_channel)
+ schedule_alert_query.apply_async(args=args, kwargs=kwargs)
+
def post_update(self, item: "AlertModelView") -> None:
self.post_add(item)
diff --git a/tests/alerts_tests.py b/tests/alerts_tests.py
index 53245e6..6a453fd 100644
--- a/tests/alerts_tests.py
+++ b/tests/alerts_tests.py
@@ -259,11 +259,6 @@ def test_operator_validator(setup_database):
operator_validator(alert1.sql_observer[0], '{"op": ">=", "threshold": 60}')
is False
)
- # ensure that 0 threshold works
- assert (
- operator_validator(alert1.sql_observer[0], '{"op": ">=", "threshold": 0}')
- is False
- )
# Test passing SQLObserver with result that doesn't pass a greater than threshold
alert2 = create_alert(dbsession, "SELECT 55")
@@ -356,7 +351,7 @@ def test_deliver_alert_screenshot(
"initial_comment": f"\n*Triggered Alert: {alert.label} :redalert:*\n"
f"*Query*:```{alert.sql_observer[0].sql}```\n"
f"*Result*: {alert.observations[-1].value}\n"
- f"*Reason*: {alert.observations[-1].value} {alert.validators[0].pretty_config}\n"
+ f"*Reason*: {alert.observations[-1].value} {alert.validators[0].pretty_print()}\n"
f"<http://0.0.0.0:8080/alert/show/{alert.id}"
f"|View Alert Details>\n<http://0.0.0.0:8080/superset/slice/{alert.slice_id}/"
"|*Explore in Superset*>",