feat(simple_metrics): warn when frequency_alert is below the 1m floor (SIENTIAPDE-2190)
This commit is contained in:
@@ -20,6 +20,7 @@ with workflow.unsafe.imports_passed_through():
|
|||||||
from orchestrator.utils.orchestrator_functions import (
|
from orchestrator.utils.orchestrator_functions import (
|
||||||
build_tag_config,
|
build_tag_config,
|
||||||
drift,
|
drift,
|
||||||
|
frequency_alert_warning,
|
||||||
gather_read_tags,
|
gather_read_tags,
|
||||||
minimal_retrain,
|
minimal_retrain,
|
||||||
pi_web_api_scouter,
|
pi_web_api_scouter,
|
||||||
@@ -409,6 +410,24 @@ class Formatters(SientiaMonitoring):
|
|||||||
schedules, current_schedules, to_update, to_create, namespace, metadata
|
schedules, current_schedules, to_update, to_create, namespace, metadata
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Only schedules being created/updated, so this fires once per config change.
|
||||||
|
changed = {**to_create[self.laborious_namespace], **to_update[self.laborious_namespace]}
|
||||||
|
for schedule_name, schedule in changed.items():
|
||||||
|
warning = frequency_alert_warning(schedule.get('frequency_alert'))
|
||||||
|
if warning:
|
||||||
|
self.send_notification(
|
||||||
|
metadata={
|
||||||
|
**metadata,
|
||||||
|
'schedule_name': schedule_name,
|
||||||
|
'model_id': schedule.get('model_id'),
|
||||||
|
'model_name': schedule.get('model_name'),
|
||||||
|
},
|
||||||
|
notification_id='SIMPLE_METRICS_FREQUENCY_ALERT_BELOW_FLOOR',
|
||||||
|
message=warning,
|
||||||
|
block='create_schedule_config',
|
||||||
|
level=NotificationLevel.WARNING,
|
||||||
|
)
|
||||||
|
|
||||||
for namespace, schedules in current_schedule_config.items():
|
for namespace, schedules in current_schedule_config.items():
|
||||||
for schedule_name in schedules:
|
for schedule_name in schedules:
|
||||||
if schedule_name not in schedule_config[namespace]:
|
if schedule_name not in schedule_config[namespace]:
|
||||||
|
|||||||
@@ -115,6 +115,27 @@ def simple_metrics(config: dict[str, Any]):
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
MIN_FREQUENCY_ALERT_SECONDS = 60
|
||||||
|
|
||||||
|
|
||||||
|
def frequency_alert_warning(frequency_alert: str | None) -> str | None:
|
||||||
|
"""
|
||||||
|
Warning message when `frequency_alert` is below the 1-minute floor or does not parse,
|
||||||
|
otherwise None. Warn only: the configured value is kept (SIENTIAPDE-1995 decision 7).
|
||||||
|
"""
|
||||||
|
if not frequency_alert:
|
||||||
|
return None
|
||||||
|
try:
|
||||||
|
# str(): a non-string value (e.g. a number typed into the doc) must warn, not raise
|
||||||
|
# AttributeError and fail the whole create_schedule_config activity.
|
||||||
|
seconds = parse_frequency(str(frequency_alert))
|
||||||
|
except ValueError:
|
||||||
|
return f'frequency_alert={frequency_alert!r} is not a valid frequency'
|
||||||
|
if seconds < MIN_FREQUENCY_ALERT_SECONDS:
|
||||||
|
return f'frequency_alert={frequency_alert} is below the 1m floor; kept as configured'
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
def minimal_retrain(config: dict[str, Any]):
|
def minimal_retrain(config: dict[str, Any]):
|
||||||
"""
|
"""
|
||||||
Build minimal retrain configuration from pipeline config.
|
Build minimal retrain configuration from pipeline config.
|
||||||
|
|||||||
@@ -1023,3 +1023,41 @@ def test_filter_notification_reports_ignore_models_id_explicit_none_does_not_rai
|
|||||||
response = formatters.filter_notification_reports(input_data)
|
response = formatters.filter_notification_reports(input_data)
|
||||||
|
|
||||||
assert response['test_group_1']['notifications'] == input_data['notification_package']
|
assert response['test_group_1']['notifications'] == input_data['notification_package']
|
||||||
|
|
||||||
|
|
||||||
|
def _schedule_config_with_frequency_alert(frequency_alert):
|
||||||
|
return {
|
||||||
|
'metadata': {},
|
||||||
|
'current_schedule_config': {'laborious': {}},
|
||||||
|
'schedule_config': {
|
||||||
|
'laborious': {
|
||||||
|
'm1-simple_metrics-60m': {
|
||||||
|
'workflow_type': 'simple_metrics',
|
||||||
|
'model_id': 'm1',
|
||||||
|
'model_name': 'm1',
|
||||||
|
'frequency': '1h',
|
||||||
|
'frequency_alert': frequency_alert,
|
||||||
|
'updated_at': '2026-10-07',
|
||||||
|
}
|
||||||
|
},
|
||||||
|
'scouter': {},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_schedule_config_warns_on_frequency_alert_below_floor(formatters):
|
||||||
|
result = formatters.create_schedule_config(_schedule_config_with_frequency_alert('30s'))
|
||||||
|
|
||||||
|
# The configured value is kept.
|
||||||
|
assert result['to_create']['laborious']['m1-simple_metrics-60m']['frequency_alert'] == '30s'
|
||||||
|
formatters.send_notification.assert_called_once()
|
||||||
|
kwargs = formatters.send_notification.call_args.kwargs
|
||||||
|
assert kwargs['notification_id'] == 'SIMPLE_METRICS_FREQUENCY_ALERT_BELOW_FLOOR'
|
||||||
|
assert kwargs['level'] == NotificationLevel.WARNING
|
||||||
|
assert kwargs['metadata']['schedule_name'] == 'm1-simple_metrics-60m'
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_schedule_config_no_warning_at_or_above_floor(formatters):
|
||||||
|
formatters.create_schedule_config(_schedule_config_with_frequency_alert('5m'))
|
||||||
|
|
||||||
|
formatters.send_notification.assert_not_called()
|
||||||
|
|||||||
@@ -1,10 +1,13 @@
|
|||||||
from unittest.mock import call, patch
|
from unittest.mock import call, patch
|
||||||
|
|
||||||
|
from pytest import mark
|
||||||
|
|
||||||
from orchestrator.utils.orchestrator_functions import (
|
from orchestrator.utils.orchestrator_functions import (
|
||||||
base_scouter,
|
base_scouter,
|
||||||
build_tag_config,
|
build_tag_config,
|
||||||
common_config,
|
common_config,
|
||||||
drift,
|
drift,
|
||||||
|
frequency_alert_warning,
|
||||||
gather_read_tags,
|
gather_read_tags,
|
||||||
minimal_retrain,
|
minimal_retrain,
|
||||||
overlap_filter_config,
|
overlap_filter_config,
|
||||||
@@ -800,3 +803,19 @@ def test_pi_web_api_scouter_with_no_timeout():
|
|||||||
'runtime': 'legacy',
|
'runtime': 'legacy',
|
||||||
}
|
}
|
||||||
assert result == expected
|
assert result == expected
|
||||||
|
|
||||||
|
|
||||||
|
@mark.parametrize(
|
||||||
|
'frequency_alert, warns',
|
||||||
|
[
|
||||||
|
(None, False),
|
||||||
|
('5m', False),
|
||||||
|
('1m', False), # the floor itself is allowed
|
||||||
|
('60s', False),
|
||||||
|
('30s', True),
|
||||||
|
('5min', True), # unparseable
|
||||||
|
(5, True), # not a string: must warn, never raise
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_frequency_alert_warning(frequency_alert, warns):
|
||||||
|
assert (frequency_alert_warning(frequency_alert) is not None) is warns
|
||||||
|
|||||||
Reference in New Issue
Block a user