remove other notification actions (#64)

* remove other notification actions

details:
- it has been quite a while since we only support GlobalActions
- this PR removes both the helper methods to turn models into GlobalActions
as well as typing references to other models
- it might break existing customers using the tuned models, so the next release
should communicate this properly
- this PR also fixes a bunch of typing issues after running pyright on the repo
- remove unused tests + use Annotated type

pyproject.toml

@@ -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"]

tests/monitor/manager/test_manager.py

@@ -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="[email protected]")]
+        monitor_setup.actions = [GlobalAction(target="some_long_id")]
         monitor_setup.config.mode = "weird_mode" # type: ignore
         monitor_setup.apply()
 
@@ -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='[email protected]'),
-            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': '[email protected]'}
-            ),
-            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()

whylabs_toolkit/helpers/monitor_helpers.py

@@ -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

whylabs_toolkit/helpers/schema.py

@@ -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()

whylabs_toolkit/monitor/manager/manager.py

@@ -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)
@@ -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,

whylabs_toolkit/monitor/manager/monitor_setup.py

@@ -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
@@ -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
@@ -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
 
@@ -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
@@ -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
 
@@ -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,
@@ -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,
@@ -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)
                 return None
 
             elif isinstance(self._target_matrix, DatasetMatrix) and not isinstance(
@@ -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 (
@@ -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()

whylabs_toolkit/monitor/models/__init__.py

@@ -57,11 +57,6 @@
     "CronSchedule",
     "FixedCadenceSchedule",
     "Cadence",
-    # monitor actions
-    # "RawWebhook",
-    "SlackWebhook",
-    "EmailRecipient",
-    "PagerDuty",
     "GlobalAction",
     # big document
     "Document",

whylabs_toolkit/monitor/models/analyzer/analyzer.py

@@ -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."
     )