Compare commits
11 Commits
fix/SIENTI
...
fix/SIENTI
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
b7325938f5 | ||
|
|
081298a883 | ||
|
|
a59cc1c691 | ||
|
|
6a95b8d2df | ||
|
|
86962c2cfc | ||
|
|
9df151d29d | ||
|
|
d250383f78 | ||
| a259aeeef7 | |||
|
|
3d2a5ecc43 | ||
|
|
6479737dfb | ||
|
|
58c0bd4f2d |
@@ -45,7 +45,7 @@ CI quality gate (`.github/workflows/quality-gate.yml`) calls the shared reusable
|
||||
|
||||
### Layering
|
||||
- **`orchestrator/worker/worker.py`** — entrypoint (`python -m orchestrator.worker.worker`). Builds a single `Activities` instance, connects to Temporal, starts Prometheus (`HTTP_METRICS_PORT`) and Temporal SDK metrics (`HTTP_SDK_METRICS_PORT`) servers, then runs three `Worker`s concurrently via `asyncio.gather`, each on its own task queue: `orchestrator-queue`, `alerts-queue`, `reports-queue`. `prepare_worker.py` derives the queue name from the workflow class name (`camel_to_snake`) and reads per-queue concurrency/poller tuning from env vars prefixed with the workflow name (e.g. `ORCHESTRATOR_MAX_CONCURRENT_ACTIVITIES`).
|
||||
- **`orchestrator/workflows/`** — Temporal workflow definitions: `orchestrator.py` (pipeline/slot/schedule orchestration), `alerts.py` (real-time ERROR notifications), `reports.py` (scheduled all-level reports). `alerts.py` and `reports.py` both delegate to the same two subworkflows in `workflows/subworkflows/`: `load_notification_package.py` (load notifications + receiver configs from Mongo, incremental via Redis timestamp) and `process_notifications.py` (build HTML email, send, log to Postgres). The difference between alerts and reports is which filter activity they call (`filter_notification_alerts` vs `filter_notification_reports`) and their base data filter (`level: ERROR` vs all levels).
|
||||
- **`orchestrator/workflows/`** — Temporal workflow definitions: `orchestrator.py` (pipeline/slot/schedule orchestration), `alerts.py` (real-time ERROR notifications), `reports.py` (scheduled all-level reports). `alerts.py` and `reports.py` both delegate to the same two subworkflows in `workflows/subworkflows/`: `load_notification_package.py` (load notifications + receiver configs from Mongo, incremental via Redis timestamp) and `process_notifications.py` (build HTML email, send, log to Postgres). The difference between alerts and reports is which filter activity they call (`filter_notification_alerts` vs `filter_notification_reports`) and their base data filter (`level: ERROR` by default vs all levels) — `load_notification_package.py`'s `_compute_query_time_filter` can widen either beyond that default to the union of active receiver groups' own `levels` (SIENTIAPDE-2011); the ERROR-only default only holds when no active group narrows further.
|
||||
- **`orchestrator/activities/activities.py`** — `Activities` is a single class combining every activity mixin via multiple inheritance: `TemporalManager, SlotManager, Formatters, MongoDB, Email, Postgres` (Postgres comes from `sientia_do.temporal.activities.postgres`, the external `sientia-dataops-library` dependency). One instance is constructed in `worker.py` and its bound methods are handed to each `Worker` as the activity list — activities are plain methods on this shared object, not separate classes. Each mixin's `__init__` takes its own config dict plus shared `logger`, `notification_handler`, `metrics_controller`. `Activities.shutdown()` closes every mixin's connections.
|
||||
- **`orchestrator/utils/`** — `connectors_config.py` builds each config dict from env vars (Temporal/Redis/MongoDB/Postgres/Email); `email_builder.py` renders Jinja2 templates (`utils/templates/email_template.html`, `general_template.html`); `orchestrator_functions.py` transforms pipeline documents into pipeline-type-specific configs (`scouter`, `pi_web_api_scouter`, `predictions_batch`, `minimal_retrain`, `drift`, `simple_metrics`); `converters.py` handles Temporal schedule frequency parsing.
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ import json
|
||||
import re
|
||||
import uuid
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from email.message import EmailMessage, Message
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
@@ -356,3 +357,40 @@ def default_receiver_group(
|
||||
'contents': contents or ['core_alerts', 'persistent_alerts', 'reports'],
|
||||
'ignore_models_id': ignore_models_id or [],
|
||||
}
|
||||
|
||||
|
||||
def extract_html_body(message: EmailMessage | Message) -> str:
|
||||
"""
|
||||
Return the text/html portion of an email message as a decoded string.
|
||||
|
||||
Walks every part looking for the first text/html payload, decoding it
|
||||
according to the part's transfer encoding and charset. Falls back to
|
||||
the message's own ``get_content``/raw payload when no HTML part is
|
||||
present so callers can still inspect plain-text reports. Shared between
|
||||
the Alerts and Reports e2e suites, both of which send mail through the
|
||||
same ``build_email_html``/``send_email`` activities.
|
||||
|
||||
Args:
|
||||
message: Captured email message returned by the test SMTP server.
|
||||
|
||||
Return:
|
||||
str: HTML body content, or an empty string when nothing decodable
|
||||
is found.
|
||||
"""
|
||||
if message.is_multipart():
|
||||
for part in message.walk():
|
||||
if part.get_content_type() != 'text/html':
|
||||
continue
|
||||
payload = part.get_payload(decode=True)
|
||||
if payload is None:
|
||||
continue
|
||||
charset = part.get_content_charset() or 'utf-8'
|
||||
return payload.decode(charset, errors='replace')
|
||||
payload = message.get_payload(decode=True)
|
||||
if payload is not None:
|
||||
charset = message.get_content_charset() or 'utf-8'
|
||||
return payload.decode(charset, errors='replace')
|
||||
try:
|
||||
return message.get_content()
|
||||
except (AttributeError, KeyError):
|
||||
return str(message.get_payload() or '')
|
||||
|
||||
@@ -71,12 +71,12 @@ Alert past `notification_ttl` with cache cleared → new email sent.
|
||||
|
||||
### A.2.1 Group filtering
|
||||
|
||||
Receiver group `levels` / `ignore_models_id` honored. The query-time half
|
||||
(union of `levels`, intersection of `ignore_models_id` across active groups)
|
||||
is covered at the subworkflow level — see `LoadNotificationPackage` below.
|
||||
The per-group half (each group only receiving its own precise slice) is not
|
||||
yet implemented in production code (SIENTIAPDE-2012) and so is not yet
|
||||
exercised here.
|
||||
Receiver group `levels` / `ignore_models_id` honored, both halves now covered:
|
||||
the query-time union/intersection across active groups (SIENTIAPDE-2011, see
|
||||
`LoadNotificationPackage` below) and the per-group precise slice
|
||||
(SIENTIAPDE-2012) — a group only receives notifications matching its own
|
||||
`levels`/`ignore_models_id`, even when the query-time filter widened the fetch
|
||||
to satisfy another active group's broader configuration.
|
||||
|
||||
### A.3.1 Empty queue
|
||||
|
||||
|
||||
@@ -12,6 +12,7 @@ from e2e.helpers import (
|
||||
count_log_report_rows,
|
||||
default_notification,
|
||||
default_receiver_group,
|
||||
extract_html_body,
|
||||
fetch_log_report,
|
||||
load_scenario_input,
|
||||
make_workflow_id,
|
||||
@@ -92,6 +93,95 @@ async def test_scenario_a_1_2_duplicate_suppressed(
|
||||
assert count_log_report_rows(postgres_engine, 'Alerts') == 0
|
||||
|
||||
|
||||
@pytest.mark.e2e
|
||||
@pytest.mark.asyncio
|
||||
async def test_scenario_a_2_1_per_group_levels_and_ignore_models_id(
|
||||
temporal_env: WorkflowEnvironment,
|
||||
orchestrator_worker,
|
||||
stub_workers,
|
||||
mongo_uri: str,
|
||||
smtp_server,
|
||||
):
|
||||
"""
|
||||
A.2.1 (SIENTIAPDE-2012): each receiver group gets only its own precise
|
||||
slice of notifications, on top of SIENTIAPDE-2011's query-time fetch.
|
||||
|
||||
Two active groups widen the query-time union beyond Alerts' ERROR-only
|
||||
default: group-errors (levels=['ERROR']) and group-warnings
|
||||
(levels=['WARNING'], ignore_models_id=['model-x']). Because the query-time
|
||||
filter only unions/intersects across ALL active groups, both ERROR and
|
||||
WARNING notifications get fetched for every group, and model-x is not
|
||||
excluded at the query level either (group-errors has no ignore_models_id
|
||||
restriction, so the intersection is empty). The per-group filter added in
|
||||
this subtask is what actually narrows each group back down to what it
|
||||
configured.
|
||||
"""
|
||||
seed_receiver_groups(
|
||||
mongo_uri,
|
||||
E2E_DATABASE,
|
||||
[
|
||||
default_receiver_group(
|
||||
group_name='group-errors', contents=['core_alerts'], levels=['ERROR']
|
||||
),
|
||||
default_receiver_group(
|
||||
group_name='group-warnings',
|
||||
contents=['core_alerts'],
|
||||
levels=['WARNING'],
|
||||
ignore_models_id=['model-x'],
|
||||
),
|
||||
],
|
||||
)
|
||||
seed_notifications(
|
||||
mongo_uri,
|
||||
E2E_DATABASE,
|
||||
[
|
||||
default_notification(
|
||||
notification_id='err-a', level='ERROR', model_name='model-a', model_id='model-a'
|
||||
),
|
||||
default_notification(
|
||||
notification_id='warn-a',
|
||||
level='WARNING',
|
||||
model_name='model-a',
|
||||
model_id='model-a',
|
||||
timestamp=datetime(2024, 6, 1, 12, 1, 0, tzinfo=UTC),
|
||||
),
|
||||
default_notification(
|
||||
notification_id='warn-x',
|
||||
level='WARNING',
|
||||
model_name='model-x',
|
||||
model_id='model-x',
|
||||
timestamp=datetime(2024, 6, 1, 12, 2, 0, tzinfo=UTC),
|
||||
),
|
||||
],
|
||||
)
|
||||
|
||||
await start_and_await_workflow(
|
||||
temporal_env.client,
|
||||
Alerts.run,
|
||||
load_scenario_input('alerts_happy_path.json'),
|
||||
make_workflow_id('alerts-2012'),
|
||||
)
|
||||
|
||||
# One email per group -- each group ended up with exactly one notification.
|
||||
assert len(smtp_server.messages) == 2
|
||||
bodies = [extract_html_body(message) for message in smtp_server.messages]
|
||||
combined = '\n'.join(bodies)
|
||||
|
||||
# group-errors (levels=['ERROR']): received err-a, never warn-a/warn-x
|
||||
# despite both being fetched by the widened query-time union.
|
||||
errors_body = next(b for b in bodies if 'Errors detected:' in b)
|
||||
assert 'Warnings detected:' not in errors_body
|
||||
|
||||
# group-warnings (levels=['WARNING'], ignore_models_id=['model-x']):
|
||||
# received warn-a only -- warn-x was fetched (query-time intersection is
|
||||
# empty, since group-errors doesn't ignore any model) but dropped here by
|
||||
# the per-group ignore_models_id check.
|
||||
warnings_body = next(b for b in bodies if 'Warnings detected:' in b)
|
||||
assert 'Errors detected:' not in warnings_body
|
||||
assert 'model-a' in combined
|
||||
assert 'model-x' not in combined
|
||||
|
||||
|
||||
@pytest.mark.e2e
|
||||
@pytest.mark.asyncio
|
||||
async def test_scenario_a_3_1_empty_queue(
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
"""E2E tests for the Reports main workflow."""
|
||||
|
||||
from datetime import UTC, datetime
|
||||
from email.message import EmailMessage, Message
|
||||
|
||||
import pytest
|
||||
from temporalio.testing import WorkflowEnvironment
|
||||
@@ -11,6 +10,7 @@ from e2e.helpers import (
|
||||
count_log_report_rows,
|
||||
default_notification,
|
||||
default_receiver_group,
|
||||
extract_html_body,
|
||||
load_scenario_input,
|
||||
make_workflow_id,
|
||||
seed_notifications,
|
||||
@@ -20,41 +20,6 @@ from e2e.helpers import (
|
||||
from orchestrator.workflows.reports import Reports
|
||||
|
||||
|
||||
def _extract_html_body(message: EmailMessage | Message) -> str:
|
||||
"""
|
||||
Return the text/html portion of an email message as a decoded string.
|
||||
|
||||
Walks every part looking for the first text/html payload, decoding it
|
||||
according to the part's transfer encoding and charset. Falls back to
|
||||
the message's own ``get_content``/raw payload when no HTML part is
|
||||
present so callers can still inspect plain-text reports.
|
||||
|
||||
Args:
|
||||
message: Captured email message returned by the test SMTP server.
|
||||
|
||||
Return:
|
||||
str: HTML body content, or an empty string when nothing decodable
|
||||
is found.
|
||||
"""
|
||||
if message.is_multipart():
|
||||
for part in message.walk():
|
||||
if part.get_content_type() != 'text/html':
|
||||
continue
|
||||
payload = part.get_payload(decode=True)
|
||||
if payload is None:
|
||||
continue
|
||||
charset = part.get_content_charset() or 'utf-8'
|
||||
return payload.decode(charset, errors='replace')
|
||||
payload = message.get_payload(decode=True)
|
||||
if payload is not None:
|
||||
charset = message.get_content_charset() or 'utf-8'
|
||||
return payload.decode(charset, errors='replace')
|
||||
try:
|
||||
return message.get_content()
|
||||
except (AttributeError, KeyError):
|
||||
return str(message.get_payload() or '')
|
||||
|
||||
|
||||
@pytest.mark.e2e
|
||||
@pytest.mark.asyncio
|
||||
async def test_scenario_r_1_1_happy_path(
|
||||
@@ -99,7 +64,7 @@ async def test_scenario_r_1_1_happy_path(
|
||||
)
|
||||
|
||||
assert len(smtp_server.messages) == 1
|
||||
body = _extract_html_body(smtp_server.messages[0])
|
||||
body = extract_html_body(smtp_server.messages[0])
|
||||
assert 'Errors detected:' in body
|
||||
assert 'Warnings detected:' in body
|
||||
assert 'Infos detected:' in body
|
||||
@@ -138,7 +103,7 @@ async def test_scenario_r_1_2_error_section_only(
|
||||
)
|
||||
|
||||
assert smtp_server.messages, 'Expected at least one report email'
|
||||
body = _extract_html_body(smtp_server.messages[0])
|
||||
body = extract_html_body(smtp_server.messages[0])
|
||||
assert 'Errors detected:' in body
|
||||
assert 'Warnings detected:' not in body
|
||||
assert 'Infos detected:' not in body
|
||||
|
||||
@@ -7,11 +7,15 @@ with workflow.unsafe.imports_passed_through():
|
||||
from math import ceil
|
||||
from typing import Any, TypedDict, cast
|
||||
|
||||
from pandas import DataFrame
|
||||
from pandas import DataFrame, to_datetime
|
||||
from sientia_do.notifications.handlers import CoreNotificationHandler as NotificationHandler
|
||||
from sientia_do.notifications.models import NotificationLevel
|
||||
from sientia_do.observability.sientia_monitoring import MetricsController, SientiaMonitoring
|
||||
from sientia_do.temporal.constants import DATETIME_FORMAT_MS_WITH_TZ, now
|
||||
from sientia_do.temporal.constants import (
|
||||
DATETIME_FORMAT_MS_WITH_TZ,
|
||||
DATETIME_FORMAT_WITH_TZ,
|
||||
now,
|
||||
)
|
||||
|
||||
from orchestrator.utils.orchestrator_functions import (
|
||||
build_tag_config,
|
||||
@@ -755,9 +759,24 @@ class Formatters(SientiaMonitoring):
|
||||
key = f'{notification_id}:{trigger}'
|
||||
|
||||
if key not in data:
|
||||
# ``notification['timestamp']`` started out as a native
|
||||
# ``datetime``/BSON Date in Mongo, but by the time it gets here
|
||||
# it has crossed at least one Temporal activity boundary:
|
||||
# Temporal's default data converter serializes ``datetime``
|
||||
# values to plain ISO-8601 strings and does not restore them on
|
||||
# decode, and pymongo returns naive datetimes for BSON Date
|
||||
# fields (no tzinfo), so the ISO string may carry no UTC offset
|
||||
# at all. Normalizing with ``utc=True`` and re-formatting to
|
||||
# ``DATETIME_FORMAT_WITH_TZ`` (same pattern as
|
||||
# ``SlotManager.put_last_data_timestamp``) keeps the value a
|
||||
# fixed-format string that ``export_data_to_postgres``'s
|
||||
# ``timestamp_conversion`` can parse after the next boundary
|
||||
# crossing, regardless of what shape it arrived in here.
|
||||
data[key] = {
|
||||
'status': group_config['status'],
|
||||
'timestamp': notification['timestamp'],
|
||||
'timestamp': to_datetime(notification['timestamp'], utc=True).strftime(
|
||||
DATETIME_FORMAT_WITH_TZ
|
||||
),
|
||||
'groups': [group_name],
|
||||
'message': notification['message'],
|
||||
'level': notification['level'],
|
||||
@@ -805,6 +824,8 @@ class Formatters(SientiaMonitoring):
|
||||
- group_name (str): Name of the receiver group
|
||||
- contents (list): Content types to include (must contain "reports")
|
||||
- ignore (list, optional): Notification IDs to exclude
|
||||
- levels (list, optional): Notification levels to include
|
||||
- ignore_models_id (list, optional): Model IDs to exclude
|
||||
|
||||
Returns:
|
||||
dict[str, Any]: Filtered receiver groups with their notifications, keyed by group_name.
|
||||
@@ -828,6 +849,8 @@ class Formatters(SientiaMonitoring):
|
||||
already_added_keys = []
|
||||
|
||||
ignore_list = receiver_group.get('ignore', [])
|
||||
levels_list = receiver_group.get('levels')
|
||||
ignore_models_id_list = receiver_group.get('ignore_models_id') or []
|
||||
|
||||
for notification in notification_package:
|
||||
alert_type = 'reports'
|
||||
@@ -840,6 +863,11 @@ class Formatters(SientiaMonitoring):
|
||||
alert_type in receiver_group['contents']
|
||||
and notification_id not in ignore_list
|
||||
and key not in already_added_keys
|
||||
and (not levels_list or notification.get('level') in levels_list)
|
||||
and (
|
||||
not ignore_models_id_list
|
||||
or notification.get('model_id') not in ignore_models_id_list
|
||||
)
|
||||
):
|
||||
receiver_groups[group_name]['notifications'].append(notification)
|
||||
already_added_keys.append(key)
|
||||
|
||||
@@ -396,6 +396,8 @@ class SlotManager(SientiaMonitoring):
|
||||
- group_name (str): Name of the receiver group
|
||||
- contents (list): Alert types to include (core_alerts, persistent_alerts)
|
||||
- ignore (list, optional): Notification IDs to exclude
|
||||
- levels (list, optional): Notification levels to include
|
||||
- ignore_models_id (list, optional): Model IDs to exclude
|
||||
- notification_ttl (int): Seconds before considering notification persistent
|
||||
|
||||
Returns:
|
||||
@@ -415,48 +417,118 @@ class SlotManager(SientiaMonitoring):
|
||||
|
||||
for receiver_group in sending_configs:
|
||||
group_name = receiver_group['group_name']
|
||||
receiver_groups[group_name] = {**receiver_group, 'notifications': []}
|
||||
receiver_groups[group_name]['notifications'] = []
|
||||
|
||||
already_added_keys = []
|
||||
|
||||
ignore_list = receiver_group.get('ignore', [])
|
||||
|
||||
for notification in notification_package:
|
||||
alert_type = 'do_nothing'
|
||||
notification_id = notification['notification_id']
|
||||
# Check if notification was recently sent
|
||||
key = f'{notification["trigger"]}:{notification_id}'
|
||||
|
||||
last_sent = self.redis_repository.get(key)
|
||||
|
||||
if last_sent is None:
|
||||
alert_type = 'core_alerts'
|
||||
|
||||
else:
|
||||
last_sent = datetime.strptime(last_sent, DATETIME_FORMAT_MS_WITH_TZ)
|
||||
|
||||
# Check if "notification_ttl" seconds has passed since last sent
|
||||
if (now() - last_sent) > timedelta(seconds=notification_ttl):
|
||||
alert_type = 'persistent_alerts'
|
||||
|
||||
# Check if this group must be notified
|
||||
if (
|
||||
alert_type in receiver_group['contents']
|
||||
and notification_id not in ignore_list
|
||||
and key not in already_added_keys
|
||||
):
|
||||
receiver_groups[group_name]['notifications'].append(notification)
|
||||
already_added_keys.append(key)
|
||||
notifications = self._filter_group_notifications(
|
||||
receiver_group, notification_package, notification_ttl
|
||||
)
|
||||
receiver_groups[group_name] = {**receiver_group, 'notifications': notifications}
|
||||
|
||||
# Remove groups with no notifications
|
||||
receiver_groups = {
|
||||
return {
|
||||
group_name: group
|
||||
for group_name, group in receiver_groups.items()
|
||||
if group['notifications']
|
||||
}
|
||||
|
||||
return receiver_groups
|
||||
def _filter_group_notifications(
|
||||
self,
|
||||
receiver_group: dict[str, Any],
|
||||
notification_package: list[dict[str, Any]],
|
||||
notification_ttl: int,
|
||||
) -> list[dict[str, Any]]:
|
||||
"""
|
||||
Select the notifications this receiver group should get.
|
||||
|
||||
Args:
|
||||
receiver_group (dict[str, Any]): The receiver group configuration.
|
||||
notification_package (list[dict[str, Any]]): Candidate notifications.
|
||||
notification_ttl (int): Seconds before considering a notification persistent.
|
||||
|
||||
Returns:
|
||||
list[dict[str, Any]]: Notifications matching this group's filters.
|
||||
"""
|
||||
already_added_keys: list[str] = []
|
||||
ignore_list = receiver_group.get('ignore', [])
|
||||
levels_list = receiver_group.get('levels')
|
||||
ignore_models_id_list = receiver_group.get('ignore_models_id') or []
|
||||
|
||||
notifications = []
|
||||
for notification in notification_package:
|
||||
notification_id = notification['notification_id']
|
||||
key = f'{notification["trigger"]}:{notification_id}'
|
||||
alert_type = self._determine_alert_type(key, notification_ttl)
|
||||
|
||||
if (
|
||||
alert_type in receiver_group['contents']
|
||||
and self._check_keys_and_notif_id(
|
||||
notification_id, ignore_list, key, already_added_keys
|
||||
)
|
||||
and self._check_levels_and_ignore_models_id(
|
||||
notification, levels_list, ignore_models_id_list
|
||||
)
|
||||
):
|
||||
notifications.append(notification)
|
||||
already_added_keys.append(key)
|
||||
|
||||
return notifications
|
||||
|
||||
def _determine_alert_type(self, key: str, notification_ttl: int) -> str:
|
||||
"""
|
||||
Classify a notification as new, persistent, or not-yet-due.
|
||||
|
||||
Args:
|
||||
key (str): The Redis cache key for this notification.
|
||||
notification_ttl (int): Seconds before considering a notification persistent.
|
||||
|
||||
Returns:
|
||||
str: 'core_alerts', 'persistent_alerts', or 'do_nothing'.
|
||||
"""
|
||||
last_sent = self.redis_repository.get(key)
|
||||
if last_sent is None:
|
||||
return 'core_alerts'
|
||||
|
||||
last_sent = datetime.strptime(last_sent, DATETIME_FORMAT_MS_WITH_TZ)
|
||||
if (now() - last_sent) > timedelta(seconds=notification_ttl):
|
||||
return 'persistent_alerts'
|
||||
|
||||
return 'do_nothing'
|
||||
|
||||
@staticmethod
|
||||
def _check_levels_and_ignore_models_id(notification, levels_list, ignore_models_id_list):
|
||||
"""
|
||||
Check if the notification's level is in the levels_list and its model_id is not in the ignore_models_id_list.
|
||||
|
||||
Args:
|
||||
notification (dict): The notification to check.
|
||||
levels_list (list, optional): List of levels to include. If None, all levels are included.
|
||||
ignore_models_id_list (list, optional): List of model_ids to exclude. If None, no models are excluded.
|
||||
|
||||
Returns:
|
||||
bool: True if the notification passes the checks, False otherwise.
|
||||
"""
|
||||
level_check = not levels_list or notification.get('level') in levels_list
|
||||
model_id_check = (
|
||||
not ignore_models_id_list or notification.get('model_id') not in ignore_models_id_list
|
||||
)
|
||||
return level_check and model_id_check
|
||||
|
||||
@staticmethod
|
||||
def _check_keys_and_notif_id(
|
||||
notification_id, ignore_list, key, already_added_keys: list[str]
|
||||
) -> bool:
|
||||
"""
|
||||
Check if the key has already been added to the list of already added keys.
|
||||
|
||||
Args:
|
||||
notification_id (str): The ID of the notification.
|
||||
ignore_list (list): List of notification IDs to ignore.
|
||||
key (str): The key to check.
|
||||
already_added_keys (list[str]): List of keys that have already been added.
|
||||
|
||||
Returns:
|
||||
bool: True if the key has not been added yet, False otherwise.
|
||||
|
||||
"""
|
||||
return notification_id not in ignore_list and key not in already_added_keys
|
||||
|
||||
@activity.defn(name='store_notification_cache')
|
||||
def store_notification_cache(self, input_data: dict[str, Any]) -> None:
|
||||
|
||||
@@ -122,6 +122,17 @@ class LoadNotificationPackage:
|
||||
restriction (missing/empty `levels`), that group wants every level, so
|
||||
the query can't restrict levels at all -> falls back to whatever level
|
||||
restriction `default_filter` itself carries (e.g. ERROR-only for Alerts).
|
||||
|
||||
When every active group DOES restrict `levels`, the union REPLACES
|
||||
`default_filter`'s own level restriction rather than ANDing with it —
|
||||
this is deliberate, not an oversight: SIENTIAPDE-2011's design doc and
|
||||
ticket text both call for Alerts' hard-coded `level: ERROR` to become
|
||||
"whatever levels the active groups actually care about", falling back
|
||||
to ERROR-only only when no group narrows further. Until SIENTIAPDE-2012
|
||||
(per-group `levels` filtering) lands, this means a single active group
|
||||
configuring `levels` to include WARNING/INFO widens the Alerts fetch for
|
||||
every active group, not just that one — an accepted interim gap, not a
|
||||
bug in this function.
|
||||
- ignore_models_id: intersection of every group's `ignore_models_id`. A model
|
||||
is only safe to drop from the fetch if EVERY active group ignores it.
|
||||
A group with no `ignore_models_id` is treated as "ignores nothing" (empty
|
||||
@@ -141,8 +152,11 @@ class LoadNotificationPackage:
|
||||
else:
|
||||
query_filter = dict(default_filter)
|
||||
|
||||
# `or []`, not a `.get(..., [])` default: a document with `ignore_models_id`
|
||||
# explicitly set to `null` (no schema validation on this collection) makes
|
||||
# `.get()` return None, and `set(None)` would raise TypeError.
|
||||
sending_configs_ignore_models_id = [
|
||||
set(group.get('ignore_models_id', [])) for group in sending_configs
|
||||
set(group.get('ignore_models_id') or []) for group in sending_configs
|
||||
]
|
||||
models_intersection = set.intersection(*sending_configs_ignore_models_id)
|
||||
|
||||
|
||||
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
|
||||
|
||||
[project]
|
||||
name = "orchestrator"
|
||||
version = "0.6.3"
|
||||
version = "0.6.4"
|
||||
description = "Sientia DataOps Orchestrator - ML Model Orchestration System"
|
||||
readme = "README.md"
|
||||
requires-python = ">=3.11"
|
||||
|
||||
@@ -716,7 +716,7 @@ def test_format_log_report(formatters):
|
||||
[
|
||||
{
|
||||
'status': 'sent',
|
||||
'timestamp': '2021-01-01',
|
||||
'timestamp': '2021-01-01 00:00:00+0000',
|
||||
'groups': ['test_receiver_group', 'test_receiver_group2'],
|
||||
'message': 'test_message',
|
||||
'level': 'test_level',
|
||||
@@ -766,3 +766,143 @@ def test_filter_notification_reports(formatters):
|
||||
],
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
def test_filter_notification_reports_levels_restriction(formatters):
|
||||
# SIENTIAPDE-2012: a group with `levels` set only receives notifications
|
||||
# matching one of those levels.
|
||||
input_data = {
|
||||
**metadata,
|
||||
'notification_package': [
|
||||
{
|
||||
'trigger': 'test_trigger_1',
|
||||
'notification_id': 'test_notification_id_1',
|
||||
'level': 'ERROR',
|
||||
},
|
||||
{
|
||||
'trigger': 'test_trigger_2',
|
||||
'notification_id': 'test_notification_id_2',
|
||||
'level': 'INFO',
|
||||
},
|
||||
],
|
||||
'sending_configs': [
|
||||
{'group_name': 'test_group_1', 'contents': ['reports'], 'levels': ['ERROR']},
|
||||
],
|
||||
}
|
||||
|
||||
response = formatters.filter_notification_reports(input_data)
|
||||
|
||||
assert response == {
|
||||
'test_group_1': {
|
||||
'group_name': 'test_group_1',
|
||||
'contents': ['reports'],
|
||||
'levels': ['ERROR'],
|
||||
'notifications': [
|
||||
{
|
||||
'trigger': 'test_trigger_1',
|
||||
'notification_id': 'test_notification_id_1',
|
||||
'level': 'ERROR',
|
||||
},
|
||||
],
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def test_filter_notification_reports_ignore_models_id_restriction(formatters):
|
||||
# SIENTIAPDE-2012: a group with `ignore_models_id` set does not receive
|
||||
# notifications from those models, but still receives everything else.
|
||||
input_data = {
|
||||
**metadata,
|
||||
'notification_package': [
|
||||
{
|
||||
'trigger': 'test_trigger_1',
|
||||
'notification_id': 'test_notification_id_1',
|
||||
'model_id': 'model_x',
|
||||
},
|
||||
{
|
||||
'trigger': 'test_trigger_2',
|
||||
'notification_id': 'test_notification_id_2',
|
||||
'model_id': 'model_y',
|
||||
},
|
||||
],
|
||||
'sending_configs': [
|
||||
{
|
||||
'group_name': 'test_group_1',
|
||||
'contents': ['reports'],
|
||||
'ignore_models_id': ['model_x'],
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
response = formatters.filter_notification_reports(input_data)
|
||||
|
||||
assert response == {
|
||||
'test_group_1': {
|
||||
'group_name': 'test_group_1',
|
||||
'contents': ['reports'],
|
||||
'ignore_models_id': ['model_x'],
|
||||
'notifications': [
|
||||
{
|
||||
'trigger': 'test_trigger_2',
|
||||
'notification_id': 'test_notification_id_2',
|
||||
'model_id': 'model_y',
|
||||
},
|
||||
],
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def test_filter_notification_reports_no_levels_or_ignore_models_id_passes_everything(
|
||||
formatters,
|
||||
):
|
||||
# SIENTIAPDE-2012: a group configuring neither field is unaffected -- same
|
||||
# behavior as before this subtask (only `contents`/`ignore` apply).
|
||||
input_data = {
|
||||
**metadata,
|
||||
'notification_package': [
|
||||
{
|
||||
'trigger': 'test_trigger_1',
|
||||
'notification_id': 'test_notification_id_1',
|
||||
'level': 'ERROR',
|
||||
'model_id': 'model_x',
|
||||
},
|
||||
{
|
||||
'trigger': 'test_trigger_2',
|
||||
'notification_id': 'test_notification_id_2',
|
||||
'level': 'INFO',
|
||||
'model_id': 'model_y',
|
||||
},
|
||||
],
|
||||
'sending_configs': [
|
||||
{'group_name': 'test_group_1', 'contents': ['reports']},
|
||||
],
|
||||
}
|
||||
|
||||
response = formatters.filter_notification_reports(input_data)
|
||||
|
||||
assert response['test_group_1']['notifications'] == input_data['notification_package']
|
||||
|
||||
|
||||
def test_filter_notification_reports_ignore_models_id_explicit_none_does_not_raise(
|
||||
formatters,
|
||||
):
|
||||
# receiver_groups has no schema validation -- ignore_models_id can be
|
||||
# explicitly null (key present, value None) instead of omitted. Must not
|
||||
# crash (see the equivalent SIENTIAPDE-2011 fix in load_notification_package).
|
||||
input_data = {
|
||||
**metadata,
|
||||
'notification_package': [
|
||||
{
|
||||
'trigger': 'test_trigger_1',
|
||||
'notification_id': 'test_notification_id_1',
|
||||
'model_id': 'model_x',
|
||||
},
|
||||
],
|
||||
'sending_configs': [
|
||||
{'group_name': 'test_group_1', 'contents': ['reports'], 'ignore_models_id': None},
|
||||
],
|
||||
}
|
||||
|
||||
response = formatters.filter_notification_reports(input_data)
|
||||
|
||||
assert response['test_group_1']['notifications'] == input_data['notification_package']
|
||||
|
||||
@@ -389,6 +389,162 @@ def test_filter_notification_alerts(slot_manager):
|
||||
}
|
||||
|
||||
|
||||
def test_filter_notification_alerts_levels_restriction(slot_manager):
|
||||
# SIENTIAPDE-2012: a group with `levels` set only receives notifications
|
||||
# matching one of those levels.
|
||||
slot_manager.redis_repository.get = MagicMock(side_effect=[None, None])
|
||||
|
||||
input_data = {
|
||||
**metadata,
|
||||
'notification_package': [
|
||||
{
|
||||
'trigger': 'test_trigger_1',
|
||||
'notification_id': 'test_notification_id_1',
|
||||
'level': 'ERROR',
|
||||
},
|
||||
{
|
||||
'trigger': 'test_trigger_2',
|
||||
'notification_id': 'test_notification_id_2',
|
||||
'level': 'WARNING',
|
||||
},
|
||||
],
|
||||
'sending_configs': [
|
||||
{'group_name': 'test_group_1', 'contents': ['core_alerts'], 'levels': ['ERROR']},
|
||||
],
|
||||
'notification_ttl': 300,
|
||||
'mail_type': 'test_mail_type',
|
||||
}
|
||||
|
||||
response = slot_manager.filter_notification_alerts(input_data)
|
||||
|
||||
assert response == {
|
||||
'test_group_1': {
|
||||
'group_name': 'test_group_1',
|
||||
'contents': ['core_alerts'],
|
||||
'levels': ['ERROR'],
|
||||
'notifications': [
|
||||
{
|
||||
'trigger': 'test_trigger_1',
|
||||
'notification_id': 'test_notification_id_1',
|
||||
'level': 'ERROR',
|
||||
},
|
||||
],
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def test_filter_notification_alerts_ignore_models_id_restriction(slot_manager):
|
||||
# SIENTIAPDE-2012: a group with `ignore_models_id` set does not receive
|
||||
# notifications from those models, but still receives everything else.
|
||||
slot_manager.redis_repository.get = MagicMock(side_effect=[None, None])
|
||||
|
||||
input_data = {
|
||||
**metadata,
|
||||
'notification_package': [
|
||||
{
|
||||
'trigger': 'test_trigger_1',
|
||||
'notification_id': 'test_notification_id_1',
|
||||
'model_id': 'model_x',
|
||||
},
|
||||
{
|
||||
'trigger': 'test_trigger_2',
|
||||
'notification_id': 'test_notification_id_2',
|
||||
'model_id': 'model_y',
|
||||
},
|
||||
],
|
||||
'sending_configs': [
|
||||
{
|
||||
'group_name': 'test_group_1',
|
||||
'contents': ['core_alerts'],
|
||||
'ignore_models_id': ['model_x'],
|
||||
},
|
||||
],
|
||||
'notification_ttl': 300,
|
||||
'mail_type': 'test_mail_type',
|
||||
}
|
||||
|
||||
response = slot_manager.filter_notification_alerts(input_data)
|
||||
|
||||
assert response == {
|
||||
'test_group_1': {
|
||||
'group_name': 'test_group_1',
|
||||
'contents': ['core_alerts'],
|
||||
'ignore_models_id': ['model_x'],
|
||||
'notifications': [
|
||||
{
|
||||
'trigger': 'test_trigger_2',
|
||||
'notification_id': 'test_notification_id_2',
|
||||
'model_id': 'model_y',
|
||||
},
|
||||
],
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def test_filter_notification_alerts_no_levels_or_ignore_models_id_passes_everything(
|
||||
slot_manager,
|
||||
):
|
||||
# SIENTIAPDE-2012: a group configuring neither field is unaffected -- same
|
||||
# behavior as before this subtask (only `contents`/`ignore` apply).
|
||||
slot_manager.redis_repository.get = MagicMock(side_effect=[None, None])
|
||||
|
||||
input_data = {
|
||||
**metadata,
|
||||
'notification_package': [
|
||||
{
|
||||
'trigger': 'test_trigger_1',
|
||||
'notification_id': 'test_notification_id_1',
|
||||
'level': 'ERROR',
|
||||
'model_id': 'model_x',
|
||||
},
|
||||
{
|
||||
'trigger': 'test_trigger_2',
|
||||
'notification_id': 'test_notification_id_2',
|
||||
'level': 'INFO',
|
||||
'model_id': 'model_y',
|
||||
},
|
||||
],
|
||||
'sending_configs': [
|
||||
{'group_name': 'test_group_1', 'contents': ['core_alerts']},
|
||||
],
|
||||
'notification_ttl': 300,
|
||||
'mail_type': 'test_mail_type',
|
||||
}
|
||||
|
||||
response = slot_manager.filter_notification_alerts(input_data)
|
||||
|
||||
assert response['test_group_1']['notifications'] == input_data['notification_package']
|
||||
|
||||
|
||||
def test_filter_notification_alerts_ignore_models_id_explicit_none_does_not_raise(
|
||||
slot_manager,
|
||||
):
|
||||
# receiver_groups has no schema validation -- ignore_models_id can be
|
||||
# explicitly null (key present, value None) instead of omitted. Must not
|
||||
# crash (see the equivalent SIENTIAPDE-2011 fix in load_notification_package).
|
||||
slot_manager.redis_repository.get = MagicMock(side_effect=[None])
|
||||
|
||||
input_data = {
|
||||
**metadata,
|
||||
'notification_package': [
|
||||
{
|
||||
'trigger': 'test_trigger_1',
|
||||
'notification_id': 'test_notification_id_1',
|
||||
'model_id': 'model_x',
|
||||
},
|
||||
],
|
||||
'sending_configs': [
|
||||
{'group_name': 'test_group_1', 'contents': ['core_alerts'], 'ignore_models_id': None},
|
||||
],
|
||||
'notification_ttl': 300,
|
||||
'mail_type': 'test_mail_type',
|
||||
}
|
||||
|
||||
response = slot_manager.filter_notification_alerts(input_data)
|
||||
|
||||
assert response['test_group_1']['notifications'] == input_data['notification_package']
|
||||
|
||||
|
||||
def test_store_notification_cache(slot_manager):
|
||||
"""Test store_notification_cache"""
|
||||
test_data = {
|
||||
|
||||
@@ -199,6 +199,27 @@ def test_compute_query_time_filter_mixed_groups_no_levels_restriction(load_notif
|
||||
assert result == {'level': 'ERROR'}
|
||||
|
||||
|
||||
def test_compute_query_time_filter_union_replaces_default_level_restriction(
|
||||
load_notification_package,
|
||||
):
|
||||
# default_filter already restricts level (e.g. Alerts' ERROR-only default),
|
||||
# but every active group specifies levels of its own -> the union REPLACES
|
||||
# default_filter's level restriction, it does not AND with it. This is
|
||||
# deliberate per SIENTIAPDE-2011's design doc and ticket text ("Alerts'
|
||||
# hard-coded level: ERROR should become whatever levels the active groups
|
||||
# actually care about"), not a bug -- this test locks the behavior in so a
|
||||
# future "fix" reintroducing AND semantics fails loudly instead of passing
|
||||
# silently.
|
||||
sending_configs = [
|
||||
{'group_name': 'group_a', 'levels': ['WARNING'], 'ignore_models_id': []},
|
||||
]
|
||||
default_filter = {'level': 'ERROR'}
|
||||
|
||||
result = load_notification_package._compute_query_time_filter(sending_configs, default_filter)
|
||||
|
||||
assert result == {'level': {'$in': ['WARNING']}}
|
||||
|
||||
|
||||
def test_compute_query_time_filter_model_ignored_by_only_one_group_not_excluded(
|
||||
load_notification_package,
|
||||
):
|
||||
@@ -233,3 +254,23 @@ def test_compute_query_time_filter_union_levels_and_intersection_models(
|
||||
'level': {'$in': ['ERROR', 'WARNING']},
|
||||
'model_id': {'$nin': ['model_a']},
|
||||
}
|
||||
|
||||
|
||||
def test_compute_query_time_filter_ignore_models_id_explicit_none_does_not_raise(
|
||||
load_notification_package,
|
||||
):
|
||||
# receiver_groups has no schema validation, so a document can have
|
||||
# ignore_models_id explicitly set to null (key present, value None) rather
|
||||
# than omitted. `group.get('ignore_models_id', [])` would still return None
|
||||
# in that case (the default only applies when the key is missing), and
|
||||
# `set(None)` raises TypeError -- must not crash the whole activity.
|
||||
sending_configs = [
|
||||
{'group_name': 'group_a', 'levels': ['ERROR'], 'ignore_models_id': None},
|
||||
{'group_name': 'group_b', 'levels': ['ERROR'], 'ignore_models_id': ['model_a']},
|
||||
]
|
||||
default_filter = {}
|
||||
|
||||
result = load_notification_package._compute_query_time_filter(sending_configs, default_filter)
|
||||
|
||||
assert result == {'level': {'$in': ['ERROR']}}
|
||||
assert 'model_id' not in result
|
||||
|
||||
Reference in New Issue
Block a user