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 1 commit
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"]
2 changes: 1 addition & 1 deletion whylabs_toolkit/helpers/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def require(env: ConfigVars) -> str:

@staticmethod
def get_or_default(env: ConfigVars) -> str:
val = os.getenv(env.name, env.value)
val = os.getenv(env.name, str(env.value))
if not val:
raise TypeError(f"No default value for {env.name}")
return val
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
82 changes: 41 additions & 41 deletions whylabs_toolkit/monitor/manager/monitor_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,23 @@ 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_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._target_columns: Optional[List[str]] = []
self._exclude_columns: Optional[List[str]] = []
self._monitor_tags: Optional[List[str]] = []
Expand Down Expand Up @@ -116,19 +116,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 +148,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 +236,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 +266,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 +278,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 +308,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 +336,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 +348,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
43 changes: 3 additions & 40 deletions whylabs_toolkit/monitor/models/monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from enum import Enum
from typing import Any, Dict, List, Literal, Optional, Union

from pydantic import BaseModel, Field, constr, HttpUrl, parse_obj_as, validator
from pydantic import BaseModel, Field, constr


from whylabs_toolkit.monitor.models.commons import (
Expand Down Expand Up @@ -30,43 +30,6 @@ class GlobalAction(NoExtrasBaseModel):
target: str = Field(description="The unique action ID in the platform", regex="[a-zA-Z0-9\\-_]+", max_length=100)


class EmailRecipient(NoExtrasBaseModel):
"""Action to send an email."""

type: Literal["email"] = "email"
id: str = Field(description="The e-mail ID to which you wish to send notifications to")
destination: str = Field(description="Destination email", format="email", max_length=1000)


class SlackWebhook(NoExtrasBaseModel):
"""Action to send a Slack webhook."""

type: Literal["slack"] = "slack"
id: str = Field(description="The endpoint ID to which you wish to send notifications to")
destination: str = Field(description="The Slack target webhook endpoint")


class RawWebhook(NoExtrasBaseModel):
"""Action to send a Raw webhook."""

type: Literal["raw"] = "raw"
id: str = Field(description="The endpoint ID to which you wish to send notifications to")
destination: Optional[str] = Field(
default=None, description="Sending raw unformatted message in JSON format to a webhook"
)


class PagerDuty(NoExtrasBaseModel):
"""Action to send a PagerDuty notification."""

type: Literal["pager_duty"] = "pager_duty"
id: str = Field(description="The PagerDuty endpoint ID to send notifications to")
destination: Optional[str] = Field(
default=None,
description="The secret key to access the PagerDuty endpoint. Required when the ID was not created",
)


class AnomalyFilter(NoExtrasBaseModel):
"""Filter the anomalies based on certain criteria. If the alerts are filtered down to 0, the monitor won't fire."""

Expand Down Expand Up @@ -217,7 +180,7 @@ class Monitor(NoExtrasBaseModel):
regex="[0-9a-zA-Z \\-_]+",
)
tags: Optional[ # type: ignore
List[constr(min_length=3, max_length=32, regex="[0-9a-zA-Z\\-_]")] # noqa F722
List[constr(min_length=3, max_length=32, regex="[0-9a-zA-Z\\-_]")] # type:ignore # noqa: F722
Copy link
Collaborator

@christinedraper christinedraper May 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably fix the type error here and in analyzer by changing constr to Annotated like I did with monitor schema

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just caught an error by defining analyzerIds to be a str (same as you did for the monitor schema), so I believe we should also update that there!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try changing the List[constr... on tags? So maybe type:ignore can go away

] = Field(None, description="A list of tags that are associated with the monitor.")
analyzerIds: List[constr(regex="^[A-Za-z0-9_\\-]+$")] = Field( # type: ignore # noqa: F722
title="AnalyzerIds",
Expand All @@ -234,7 +197,7 @@ class Monitor(NoExtrasBaseModel):
description="Notification mode and how we might handle different analysis",
discriminator="type",
)
actions: List[Union[GlobalAction, EmailRecipient, SlackWebhook, PagerDuty]] = Field(
actions: List[GlobalAction] = Field(
description="List of destination for the outgoing messages",
max_items=100,
)
Expand Down
Loading