Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove other notification actions #64

Merged
merged 4 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ build-backend = "poetry.core.masonry.api"

[tool.flake8]
max-line-length = 140
ignore = ["F405"]

[tool.pyright]
include = ["whylabs_toolkit/**/*.py"]

[tool.poetry.extras]
diagnoser = ["pandas", "numpy", "tabulate", "isodate", "python-dateutil"]
65 changes: 1 addition & 64 deletions tests/monitor/manager/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def test_validate(self, manager: MonitorManager) -> None:
assert manager.validate()

def test_failing_validation(self, monitor_setup: MonitorSetup) -> None:
monitor_setup.actions = [EmailRecipient(id="some_long_id", destination="someemail@email.com")]
monitor_setup.actions = [GlobalAction(target="some_long_id")]
monitor_setup.config.mode = "weird_mode" # type: ignore
monitor_setup.apply()

Expand Down Expand Up @@ -77,66 +77,3 @@ def test_monitor_running_eagerly(self, existing_monitor_setup: MonitorSetup) ->
)

assert new_expected_result["allowPartialTargetBatches"] == False


class TestNotificationActions(TestCase):
def setUp(self) -> None:
self.monitor_setup = MagicMock()
self.monitor_setup.credentials.org_id = 'test_org'

self.monitor_setup.monitor = MagicMock()
self.monitor_setup.monitor.actions = [
SlackWebhook(id='slack1', destination='https://slack.com/webhook'),
EmailRecipient(id='email1', destination='test@example.com'),
GlobalAction(target="existing-pagerDuty")
]

self.notifications_api = MagicMock()
self.notifications_api.list_notification_actions.return_value = []

self.monitor_api = MagicMock()

self.monitor_manager = MonitorManager(
setup = self.monitor_setup,
notifications_api=self.notifications_api,
monitor_api=self.monitor_api
)

def test_notification_actions_are_updated(self) -> None:
self.monitor_manager._update_notification_actions()

expected_calls = [
call(
org_id='test_org',
type='EMAIL',
action_id='email1',
body={'email': 'test@example.com'}
),
call(
org_id='test_org',
type='SLACK',
action_id='slack1',
body={'slackWebhook': 'https://slack.com/webhook'}
)
]

for call_args in expected_calls:
assert call_args in self.notifications_api.put_notification_action.call_args_list

def test_global_actions_are_made(self) -> None:
self.monitor_manager._update_notification_actions()

assert GlobalAction(target='existing-pagerDuty') in self.monitor_setup.monitor.actions

def test_existing_notification_actions_are_fetched(self) -> None:
self.monitor_manager._update_notification_actions()

self.notifications_api.list_notification_actions.assert_called_once_with(
org_id='test_org'
)

def test_error_is_raised_if_monitor_is_none(self) -> None:
self.monitor_setup.monitor = None

with self.assertRaises(ValueError):
self.monitor_manager._update_notification_actions()
2 changes: 1 addition & 1 deletion whylabs_toolkit/helpers/monitor_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,5 +122,5 @@ def delete_monitor(
resp_monitor = api.delete_monitor(org_id=org_id, dataset_id=dataset_id, monitor_id=monitor_id)
logger.debug(f"Deleted monitor with Resp:{resp_monitor}")
except ApiValueError as e:
logger.error(f"Error deleting monitor {monitor_id}: {e.msg}")
logger.error(f"Error deleting monitor {monitor_id}: {e.msg}") # type: ignore
raise e
2 changes: 1 addition & 1 deletion whylabs_toolkit/helpers/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def _update_entity_schema(self) -> None:
def _put_updated_entity_schema(self) -> None:
metadata_dict = self.current_entity_schema["metadata"]
entity_schema_dict = EntitySchema(columns=self.columns_dict, metadata=metadata_dict)
self._put_entity_schema(schema=entity_schema_dict)
self._put_entity_schema(schema=entity_schema_dict) # type: ignore

def update(self) -> None:
self._validate_input()
Expand Down
62 changes: 4 additions & 58 deletions whylabs_toolkit/monitor/manager/manager.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import logging
import json
from pathlib import Path
from typing import Optional, Union, Any, List
from typing import Optional, Any

from jsonschema import validate, ValidationError
from whylabs_client.api.notification_settings_api import NotificationSettingsApi
from whylabs_client.api.models_api import ModelsApi
from whylabs_client.api.monitor_api import MonitorApi

from whylabs_toolkit.monitor.manager.monitor_setup import MonitorSetup
from whylabs_toolkit.monitor.models import *
from whylabs_toolkit.helpers.monitor_helpers import get_model_granularity
from whylabs_toolkit.helpers.config import Config
from whylabs_toolkit.helpers.utils import get_monitor_api, get_notification_api
from whylabs_toolkit.helpers.utils import get_monitor_api


logging.basicConfig(level=logging.INFO)
Expand All @@ -23,73 +22,20 @@ def __init__(
self,
setup: MonitorSetup,
eager: Optional[bool] = None,
notifications_api: Optional[NotificationSettingsApi] = None,
monitor_api: Optional[ModelsApi] = None,
monitor_api: Optional[MonitorApi] = None,
config: Config = Config(),
) -> None:
self._setup = setup
self.__notifications_api = notifications_api or get_notification_api(config=config)
self.__monitor_api = monitor_api or get_monitor_api(config=config)
self.__eager = eager

def _get_existing_notification_actions(self) -> List[str]:
actions_dict_list = self.__notifications_api.list_notification_actions(org_id=self._setup.credentials.org_id)
action_ids = []
for action in actions_dict_list:
action_ids.append(action.get("id"))
return action_ids

@staticmethod
def get_notification_request_payload(action: Union[SlackWebhook, EmailRecipient, PagerDuty]) -> str:
if isinstance(action, SlackWebhook):
return "slackWebhook"
elif isinstance(action, EmailRecipient):
return "email"
elif isinstance(action, PagerDuty):
return "pagerDutyKey"
else:
raise ValueError(
f"Can't work with {action} type. Available options are SlackWebhook, PagerDuty and EmailRecipient."
)

def _update_notification_actions(self) -> None:
"""
Updates the notification actions to be passed to WhyLabs based on the actions defined in the MonitorBuilder object.
"""
if not self._setup.monitor:
raise ValueError("You must call apply() on your MonitorSetup object!")

existing_actions = self._get_existing_notification_actions()

for action in self._setup.monitor.actions:
if isinstance(action, GlobalAction):
continue

if action.id not in existing_actions:
logger.info(f"Didn't find a {action.type} action under the ID {action.id}, creating one now!")
payload_key = self.get_notification_request_payload(action=action)
self.__notifications_api.put_notification_action(
org_id=self._setup.credentials.org_id,
type=action.type.upper(),
action_id=action.id,
body={payload_key: action.destination},
)

if self._setup.monitor:
self._setup.monitor.actions = [
action if isinstance(action, GlobalAction) else GlobalAction(target=action.id)
for action in self._setup.monitor.actions
]

def _get_current_monitor_config(self) -> Optional[Any]:
monitor_config = self.__monitor_api.get_monitor_config_v3(
org_id=self._setup.credentials.org_id, dataset_id=self._setup.credentials.dataset_id
)
return monitor_config

def dump(self) -> Any:
self._update_notification_actions()

doc = Document(
orgId=self._setup.credentials.org_id,
datasetId=self._setup.credentials.dataset_id,
Expand Down
84 changes: 41 additions & 43 deletions whylabs_toolkit/monitor/manager/monitor_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
from datetime import datetime, timezone
from typing import Optional, List, Union, Any

from whylabs_client.exceptions import NotFoundException

from whylabs_toolkit.helpers.utils import get_models_api
from whylabs_toolkit.monitor.models import *
from whylabs_toolkit.monitor.models.analyzer.targets import ColumnGroups
Expand All @@ -30,25 +28,25 @@ def __init__(self, monitor_id: str, dataset_id: Optional[str] = None, config: Co
self._models_api = get_models_api(config=self._config)

self._monitor_mode: Optional[Union[EveryAnomalyMode, DigestMode]] = None
self._monitor_actions: Optional[List[Union[GlobalAction, EmailRecipient, SlackWebhook, PagerDuty]]] = None
self._monitor_actions: List[GlobalAction] = []
self._analyzer_schedule: Optional[Union[FixedCadenceSchedule, CronSchedule]] = None
self._target_matrix: Optional[Union[ColumnMatrix, DatasetMatrix]] = None
self._analyzer_config: Optional[
Union[
DiffConfig,
FixedThresholdsConfig,
StddevConfig,
DriftConfig,
ComparisonConfig,
SeasonalConfig,
FrequentStringComparisonConfig,
ListComparisonConfig,
ConjunctionConfig,
DisjunctionConfig,
]
] = None
self._target_columns: Optional[List[str]] = []
self._exclude_columns: Optional[List[str]] = []
self._target_matrix: Union[ColumnMatrix, DatasetMatrix] = ColumnMatrix(
include=self._target_columns, exclude=self._exclude_columns, segments=[]
)
self._analyzer_config: Union[
DiffConfig,
FixedThresholdsConfig,
StddevConfig,
DriftConfig,
ComparisonConfig,
SeasonalConfig,
FrequentStringComparisonConfig,
ListComparisonConfig,
ConjunctionConfig,
DisjunctionConfig,
]
self._monitor_tags: Optional[List[str]] = []
self._analyzer_tags: Optional[List[str]] = []
self._analyzer_disable_target_rollup: Optional[bool] = None
Expand Down Expand Up @@ -116,19 +114,17 @@ def target_matrix(self, target: Union[ColumnMatrix, DatasetMatrix]) -> None:
@property
def config(
self,
) -> Optional[
Union[
DiffConfig,
FixedThresholdsConfig,
StddevConfig,
DriftConfig,
ComparisonConfig,
SeasonalConfig,
FrequentStringComparisonConfig,
ListComparisonConfig,
ConjunctionConfig,
DisjunctionConfig,
]
) -> Union[
DiffConfig,
FixedThresholdsConfig,
StddevConfig,
DriftConfig,
ComparisonConfig,
SeasonalConfig,
FrequentStringComparisonConfig,
ListComparisonConfig,
ConjunctionConfig,
DisjunctionConfig,
]:
return self._analyzer_config

Expand All @@ -150,11 +146,11 @@ def config(
self._analyzer_config = config

@property
def actions(self) -> Optional[List[Union[GlobalAction, EmailRecipient, SlackWebhook, PagerDuty]]]:
def actions(self) -> Optional[List[GlobalAction]]:
return self._monitor_actions

@actions.setter
def actions(self, actions: List[Union[GlobalAction, EmailRecipient, SlackWebhook, PagerDuty]]) -> None:
def actions(self, actions: List[GlobalAction]) -> None:
self._monitor_actions = actions

@property
Expand Down Expand Up @@ -238,9 +234,6 @@ def set_target_columns(self, columns: List[str]) -> None:
"""
if self._validate_columns_input(columns=columns):
self._target_columns = columns
self._target_matrix = self._target_matrix or ColumnMatrix(
include=self._target_columns, exclude=self._exclude_columns, segments=[]
)
if isinstance(self._target_matrix, ColumnMatrix):
self._target_matrix.include = self._target_columns

Expand Down Expand Up @@ -271,6 +264,8 @@ def __set_analyzer(self) -> None:

self.analyzer = Analyzer(
id=self.credentials.analyzer_id,
metadata=None,
disabled=False,
displayName=self.credentials.analyzer_id,
disableTargetRollup=self._analyzer_disable_target_rollup,
targetMatrix=self._target_matrix,
Expand All @@ -281,10 +276,12 @@ def __set_analyzer(self) -> None:
)

def __set_monitor(
self, monitor_mode: Optional[Union[EveryAnomalyMode, DigestMode]], monitor_actions: Optional[List[Any]]
self, monitor_mode: Union[EveryAnomalyMode, DigestMode], monitor_actions: List[GlobalAction] = []
) -> None:
self.monitor = Monitor(
id=self.credentials.monitor_id,
metadata=None,
severity=None,
disabled=False,
displayName=self.credentials.monitor_id,
tags=self._monitor_tags,
Expand All @@ -309,7 +306,7 @@ def __set_dataset_matrix_for_dataset_metric(self) -> None:
logger.warning(
"ColumnMatrix is not configurable with a DatasetMetric." "Changing it to DatasetMatrix instead"
)
self._target_matrix = DatasetMatrix(segments=self._target_matrix.segments)
self._target_matrix = DatasetMatrix(type=TargetLevel.dataset, segments=self._target_matrix.segments)
murilommen marked this conversation as resolved.
Show resolved Hide resolved
return None

elif isinstance(self._target_matrix, DatasetMatrix) and not isinstance(
Expand Down Expand Up @@ -337,7 +334,7 @@ def __set_dataset_matrix_for_missing_data_metric(self) -> None:
"Missing data point needs to be set with target_matrix of type DatasetMatrix"
"Changing to DatasetMatrix now."
)
self._target_matrix = DatasetMatrix(segments=self._target_matrix.segments)
self._target_matrix = DatasetMatrix(type=TargetLevel.dataset, segments=self._target_matrix.segments)
return None

if (
Expand All @@ -349,18 +346,19 @@ def __set_dataset_matrix_for_missing_data_metric(self) -> None:
"secondsSinceLastUpload needs to be set with target_matrix of type DatasetMatrix"
"Changing to DatasetMatrix now."
)
self._target_matrix = DatasetMatrix(segments=self._target_matrix.segments)
self._target_matrix = DatasetMatrix(type=TargetLevel.dataset, segments=self._target_matrix.segments)
return None

def apply(self) -> None:
monitor_mode = self._monitor_mode or DigestMode()
actions = self._monitor_actions or []
monitor_mode = self._monitor_mode or DigestMode(
type="DIGEST", filter=None, creationTimeOffset=None, datasetTimestampOffset=None, groupBy=None
)
self._analyzer_schedule = self._analyzer_schedule or FixedCadenceSchedule(
cadence=get_model_granularity(
org_id=self.credentials.org_id, dataset_id=self.credentials.dataset_id # type: ignore
)
)

self.__set_monitor(monitor_mode=monitor_mode, monitor_actions=actions)
self.__set_monitor(monitor_mode=monitor_mode, monitor_actions=self._monitor_actions)

self.__set_analyzer()
5 changes: 0 additions & 5 deletions whylabs_toolkit/monitor/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,6 @@
"CronSchedule",
"FixedCadenceSchedule",
"Cadence",
# monitor actions
# "RawWebhook",
"SlackWebhook",
"EmailRecipient",
"PagerDuty",
"GlobalAction",
# big document
"Document",
Expand Down
2 changes: 1 addition & 1 deletion whylabs_toolkit/monitor/models/analyzer/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class Analyzer(NoExtrasBaseModel):
regex="[0-9a-zA-Z \\-_]+",
)
tags: Optional[ # type: ignore
List[constr(min_length=3, max_length=32, regex="[0-9a-zA-Z\\-_]")] # noqa
List[constr(min_length=3, max_length=32, regex="[0-9a-zA-Z\\-_]")] # type: ignore
] = Field( # noqa F722
None, description="A list of tags that are associated with the analyzer."
)
Expand Down
Loading