Skip to content

Commit

Permalink
Ele 1068 supperssion in cli (#1013)
Browse files Browse the repository at this point in the history
* add global alert suppression interval param to cli

* fix bug when meta value is 0

* fix test and add another

* fix bug in param name

* add missing line :)

* rename param + add default

* fix default value of suppression alert to 0

* remove const

* remove test case that never happens

* fix typing

* fix mock
  • Loading branch information
ellakz authored Jul 20, 2023
1 parent ed9435b commit 2b276ef
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 23 deletions.
12 changes: 4 additions & 8 deletions elementary/monitor/api/alerts/alerts.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from collections import defaultdict
from datetime import datetime
from typing import DefaultDict, Dict, List, Optional, Sequence, Union
from typing import DefaultDict, Dict, List, Sequence, Union

from elementary.clients.api.api_client import APIClient
from elementary.clients.dbt.dbt_runner import DbtRunner
Expand All @@ -18,16 +18,14 @@

logger = get_logger(__name__)

DEFAULT_ALERT_SUPPRESSION_INTERVAL_HOURS = 24


class AlertsAPI(APIClient):
def __init__(
self,
dbt_runner: DbtRunner,
config: Config,
elementary_database_and_schema: str,
global_suppression_interval: Optional[int] = None,
global_suppression_interval: int,
):
super().__init__(dbt_runner)
self.config = config
Expand Down Expand Up @@ -156,7 +154,7 @@ def _get_suppressed_alerts(
cls,
alerts: AlertsQueryResult[AlertType],
last_alert_sent_times: Dict[str, str],
global_suppression_interval: Optional[int] = None,
global_suppression_interval: int,
) -> List[str]:
suppressed_alerts = []
current_time_utc = datetime.utcnow()
Expand Down Expand Up @@ -219,6 +217,4 @@ def _get_latest_alerts(
def _get_suppression_interval(interval_from_alert, interval_from_cli):
if interval_from_alert is not None:
return interval_from_alert
if interval_from_cli is not None:
return interval_from_cli
return DEFAULT_ALERT_SUPPRESSION_INTERVAL_HOURS
return interval_from_cli
2 changes: 1 addition & 1 deletion elementary/monitor/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ def get_cli_properties() -> dict:
@click.option(
"--suppression-interval",
type=int,
default=24,
default=0,
help="The number of hours to suppress alerts after an alert was sent (this is a global default setting).",
)
@click.option(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def __init__(
force_update_dbt_package: bool = False,
disable_samples: bool = False,
send_test_message_on_success: bool = False,
global_suppression_interval: Optional[int] = None,
global_suppression_interval: int = 0,
):
super().__init__(
config, tracking, force_update_dbt_package, disable_samples, filter
Expand Down
5 changes: 4 additions & 1 deletion tests/mocks/api/alerts_api_mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ def __init__(self, *args, **kwargs):
mock_dbt_runner = MockDbtRunner()
config = Config()
super().__init__(
mock_dbt_runner, config, elementary_database_and_schema="test.test"
mock_dbt_runner,
config,
global_suppression_interval=0,
elementary_database_and_schema="test.test",
)
self.alerts_fetcher = MockAlertsFetcher()
19 changes: 7 additions & 12 deletions tests/unit/monitor/api/alerts/test_alerts_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@
import pytest
from parametrization import Parametrization

from elementary.monitor.api.alerts.alerts import (
DEFAULT_ALERT_SUPPRESSION_INTERVAL_HOURS,
)
from tests.mocks.api.alerts_api_mock import MockAlertsAPI


Expand All @@ -22,10 +19,14 @@ def test_get_suppressed_alerts(alerts_api_mock: MockAlertsAPI):
model_alerts = alerts_api_mock.alerts_fetcher.query_pending_model_alerts()

suppressed_test_alerts = alerts_api_mock._get_suppressed_alerts(
test_alerts, last_test_alert_sent_times
test_alerts,
last_test_alert_sent_times,
alerts_api_mock.global_suppression_interval,
)
suppressed_model_alerts = alerts_api_mock._get_suppressed_alerts(
model_alerts, last_model_alert_sent_times
model_alerts,
last_model_alert_sent_times,
alerts_api_mock.global_suppression_interval,
)

assert json.dumps(suppressed_test_alerts, sort_keys=True) == json.dumps(
Expand All @@ -49,15 +50,9 @@ def test_get_suppressed_alerts(alerts_api_mock: MockAlertsAPI):
alert_interval=10,
expected_interval=10,
)
@Parametrization.case(
name="both are none- default wins",
cli_interval=None,
alert_interval=None,
expected_interval=DEFAULT_ALERT_SUPPRESSION_INTERVAL_HOURS,
)
def test_get_suppression_interval(
alerts_api_mock: MockAlertsAPI,
cli_interval: Optional[int],
cli_interval: int,
alert_interval: Optional[int],
expected_interval: Optional[int],
):
Expand Down

0 comments on commit 2b276ef

Please sign in to comment.