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:36 UTC
[incubator-superset] branch revert-10891-bogdan/alerting_fixes
created (now 0df21d8)
This is an automated email from the ASF dual-hosted git repository.
maximebeauchemin pushed a change to branch revert-10891-bogdan/alerting_fixes
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git.
at 0df21d8 Revert "fix: superset alerting misc fixes (#10891)"
This branch includes the following new commits:
new 0df21d8 Revert "fix: superset alerting misc fixes (#10891)"
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[incubator-superset] 01/01: Revert "fix: superset alerting misc
fixes (#10891)"
Posted by ma...@apache.org.
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*>",