From d065633c81bdb77ea8b80c37502a2b94640c373d Mon Sep 17 00:00:00 2001 From: Jason Davis <32852580+JasonD28@users.noreply.github.com> Date: Tue, 28 Jul 2020 16:48:42 -0700 Subject: [PATCH] fix: Implement updates to SQL-based email alerts (#10454) * implemented updates to alerting * fixed imports and teardown * changed unittest to pytest conventions * add app_context to tests Co-authored-by: Jason Davis <@dropbox.com> --- superset/tasks/schedules.py | 7 ++- superset/views/alerts.py | 6 +- tests/alerts_tests.py | 114 ++++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 5 deletions(-) create mode 100644 tests/alerts_tests.py diff --git a/superset/tasks/schedules.py b/superset/tasks/schedules.py index 0c44f337da..3e8d6fc207 100644 --- a/superset/tasks/schedules.py +++ b/superset/tasks/schedules.py @@ -705,17 +705,18 @@ def schedule_window( for schedule in schedules: logging.info("Processing schedule %s", schedule) args = (report_type, schedule.id) + schedule_start_at = start_at if ( hasattr(schedule, "last_eval_dttm") and schedule.last_eval_dttm and schedule.last_eval_dttm > start_at ): - # start_at = schedule.last_eval_dttm + timedelta(seconds=1) - pass + schedule_start_at = schedule.last_eval_dttm + timedelta(seconds=1) + # Schedule the job for the specified time window for eta in next_schedules( - schedule.crontab, start_at, stop_at, resolution=resolution + schedule.crontab, schedule_start_at, stop_at, resolution=resolution ): get_scheduler_action(report_type).apply_async(args, eta=eta) # type: ignore break diff --git a/superset/views/alerts.py b/superset/views/alerts.py index 9dbaf29d3e..a06cf8a9b0 100644 --- a/superset/views/alerts.py +++ b/superset/views/alerts.py @@ -59,11 +59,13 @@ class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors "crontab", "database", "sql", - "alert_type", + # TODO: implement different types of alerts + # "alert_type", "owners", "recipients", "slice", - "dashboard", + # TODO: implement dashboard screenshots with alerts + # "dashboard", "log_retention", "grace_period", ) diff --git a/tests/alerts_tests.py b/tests/alerts_tests.py new file mode 100644 index 0000000000..5c08acc8bf --- /dev/null +++ b/tests/alerts_tests.py @@ -0,0 +1,114 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Unit tests for alerting in Superset""" +import logging +from unittest.mock import patch + +import pytest + +from superset import db +from superset.models.alerts import Alert, AlertLog +from superset.models.slice import Slice +from superset.tasks.schedules import run_alert_query +from superset.utils import core as utils +from tests.test_app import app + +logging.basicConfig(level=logging.INFO) +logger = logging.getLogger(__name__) + + +@pytest.yield_fixture(scope="module") +def setup_database(): + with app.app_context(): + slice_id = db.session.query(Slice).all()[0].id + database_id = utils.get_example_database().id + + alert1 = Alert( + id=1, + label="alert_1", + active=True, + crontab="*/1 * * * *", + sql="SELECT 0", + alert_type="email", + slice_id=slice_id, + database_id=database_id, + ) + alert2 = Alert( + id=2, + label="alert_2", + active=True, + crontab="*/1 * * * *", + sql="SELECT 55", + alert_type="email", + slice_id=slice_id, + database_id=database_id, + ) + alert3 = Alert( + id=3, + label="alert_3", + active=False, + crontab="*/1 * * * *", + sql="UPDATE 55", + alert_type="email", + slice_id=slice_id, + database_id=database_id, + ) + alert4 = Alert(id=4, active=False, label="alert_4", database_id=-1) + alert5 = Alert(id=5, active=False, label="alert_5", database_id=database_id) + + for num in range(1, 6): + eval(f"db.session.add(alert{num})") + db.session.commit() + yield db.session + + db.session.query(AlertLog).delete() + db.session.query(Alert).delete() + + +@patch("superset.tasks.schedules.deliver_alert") +@patch("superset.tasks.schedules.logging.Logger.error") +def test_run_alert_query(mock_error, mock_deliver, setup_database): + database = setup_database + run_alert_query(database.query(Alert).filter_by(id=1).one(), database) + alert1 = database.query(Alert).filter_by(id=1).one() + assert mock_deliver.call_count == 0 + assert len(alert1.logs) == 1 + assert alert1.logs[0].alert_id == 1 + assert alert1.logs[0].state == "pass" + + run_alert_query(database.query(Alert).filter_by(id=2).one(), database) + alert2 = database.query(Alert).filter_by(id=2).one() + assert mock_deliver.call_count == 1 + assert len(alert2.logs) == 1 + assert alert2.logs[0].alert_id == 2 + assert alert2.logs[0].state == "trigger" + + run_alert_query(database.query(Alert).filter_by(id=3).one(), database) + alert3 = database.query(Alert).filter_by(id=3).one() + assert mock_deliver.call_count == 1 + assert mock_error.call_count == 2 + assert len(alert3.logs) == 1 + assert alert3.logs[0].alert_id == 3 + assert alert3.logs[0].state == "error" + + run_alert_query(database.query(Alert).filter_by(id=4).one(), database) + assert mock_deliver.call_count == 1 + assert mock_error.call_count == 3 + + run_alert_query(database.query(Alert).filter_by(id=5).one(), database) + assert mock_deliver.call_count == 1 + assert mock_error.call_count == 4