Skip to content

Commit 7c37579

Browse files
rsareddy0329Roja Reddy Sareddy
andauthored
fix(model-monitor): honor base_job_name for job definition names (#6373)
ModelMonitor subclasses generated the monitoring job definition name from the type's JOB_DEFINITION_BASE_NAME constant, ignoring a user-supplied base_job_name -- even though the monitoring schedule name already honors it. As a result base_job_name was silently dropped for the job definition (and the monitoring resources derived from it). Add a _generate_job_definition_name() helper on the ModelMonitor base (mirroring _generate_monitoring_schedule_name) that uses base_job_name when provided and falls back to JOB_DEFINITION_BASE_NAME otherwise, and use it in the data-quality, model-quality, model-bias and model-explainability create and update paths. Also make the four *_update_failure tests deterministic: they asserted the regenerated job definition name differs from the original, which relied on two name_from_base calls landing in different milliseconds (flaky on master). Patch name_from_base with a counter-based unique name in those tests. Note: the recurring execution processing job names are assigned by the SageMaker service and remain non-customizable. Relates-to #4783 Co-authored-by: Roja Reddy Sareddy <rsareddy@amazon.com>
1 parent 10ce0b2 commit 7c37579

4 files changed

Lines changed: 100 additions & 13 deletions

File tree

‎src/sagemaker/model_monitor/clarify_model_monitoring.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
from sagemaker.model_monitor import model_monitoring as mm
2626
from sagemaker import image_uris, s3
2727
from sagemaker.session import Session
28-
from sagemaker.utils import name_from_base, format_tags
28+
from sagemaker.utils import format_tags
2929
from sagemaker.clarify import SageMakerClarifyProcessor, ModelPredictedLabelConfig
3030
from sagemaker.lineage._utils import get_resource_name_from_arn
3131

@@ -625,7 +625,7 @@ def create_monitoring_schedule(
625625
monitor_schedule_name = self._generate_monitoring_schedule_name(
626626
schedule_name=monitor_schedule_name
627627
)
628-
new_job_definition_name = name_from_base(self.JOB_DEFINITION_BASE_NAME)
628+
new_job_definition_name = self._generate_job_definition_name()
629629
request_dict = self._build_create_job_definition_request(
630630
monitoring_schedule_name=monitor_schedule_name,
631631
job_definition_name=new_job_definition_name,
@@ -768,7 +768,7 @@ def update_monitoring_schedule(
768768
job_desc = self.sagemaker_session.sagemaker_client.describe_model_bias_job_definition(
769769
JobDefinitionName=self.job_definition_name
770770
)
771-
new_job_definition_name = name_from_base(self.JOB_DEFINITION_BASE_NAME)
771+
new_job_definition_name = self._generate_job_definition_name()
772772
request_dict = self._build_create_job_definition_request(
773773
monitoring_schedule_name=self.monitoring_schedule_name,
774774
job_definition_name=new_job_definition_name,
@@ -1067,7 +1067,7 @@ def create_monitoring_schedule(
10671067
monitor_schedule_name = self._generate_monitoring_schedule_name(
10681068
schedule_name=monitor_schedule_name
10691069
)
1070-
new_job_definition_name = name_from_base(self.JOB_DEFINITION_BASE_NAME)
1070+
new_job_definition_name = self._generate_job_definition_name()
10711071
request_dict = self._build_create_job_definition_request(
10721072
monitoring_schedule_name=monitor_schedule_name,
10731073
job_definition_name=new_job_definition_name,
@@ -1215,7 +1215,7 @@ def update_monitoring_schedule(
12151215
JobDefinitionName=self.job_definition_name
12161216
)
12171217
)
1218-
new_job_definition_name = name_from_base(self.JOB_DEFINITION_BASE_NAME)
1218+
new_job_definition_name = self._generate_job_definition_name()
12191219
request_dict = self._build_create_job_definition_request(
12201220
monitoring_schedule_name=self.monitoring_schedule_name,
12211221
job_definition_name=new_job_definition_name,

‎src/sagemaker/model_monitor/model_monitoring.py‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,6 +1180,22 @@ def _generate_monitoring_schedule_name(self, schedule_name=None):
11801180

11811181
return name_from_base(base=base_name)
11821182

1183+
def _generate_job_definition_name(self):
1184+
"""Generate the monitoring job definition name.
1185+
1186+
Uses the base name given to the constructor when provided, so a
1187+
user-supplied ``base_job_name`` is reflected in the job definition
1188+
name (and the monitoring resources derived from it) rather than being
1189+
silently ignored (see #4783); otherwise falls back to the monitor
1190+
type's default base name (``JOB_DEFINITION_BASE_NAME``).
1191+
1192+
Returns:
1193+
str: The generated job definition name.
1194+
1195+
"""
1196+
base_name = self.base_job_name or self.JOB_DEFINITION_BASE_NAME
1197+
return name_from_base(base=base_name)
1198+
11831199
@staticmethod
11841200
def _generate_env_map(
11851201
env,
@@ -2023,7 +2039,7 @@ def create_monitoring_schedule(
20232039
monitor_schedule_name = self._generate_monitoring_schedule_name(
20242040
schedule_name=monitor_schedule_name
20252041
)
2026-
new_job_definition_name = name_from_base(self.JOB_DEFINITION_BASE_NAME)
2042+
new_job_definition_name = self._generate_job_definition_name()
20272043
request_dict = self._build_create_data_quality_job_definition_request(
20282044
monitoring_schedule_name=monitor_schedule_name,
20292045
job_definition_name=new_job_definition_name,
@@ -2393,7 +2409,7 @@ def _update_data_quality_monitoring_schedule(
23932409
job_desc = self.sagemaker_session.sagemaker_client.describe_data_quality_job_definition(
23942410
JobDefinitionName=self.job_definition_name
23952411
)
2396-
new_job_definition_name = name_from_base(self.JOB_DEFINITION_BASE_NAME)
2412+
new_job_definition_name = self._generate_job_definition_name()
23972413
request_dict = self._build_create_data_quality_job_definition_request(
23982414
monitoring_schedule_name=self.monitoring_schedule_name,
23992415
job_definition_name=new_job_definition_name,
@@ -3147,7 +3163,7 @@ def create_monitoring_schedule(
31473163
monitor_schedule_name = self._generate_monitoring_schedule_name(
31483164
schedule_name=monitor_schedule_name
31493165
)
3150-
new_job_definition_name = name_from_base(self.JOB_DEFINITION_BASE_NAME)
3166+
new_job_definition_name = self._generate_job_definition_name()
31513167
request_dict = self._build_create_model_quality_job_definition_request(
31523168
monitoring_schedule_name=monitor_schedule_name,
31533169
job_definition_name=new_job_definition_name,
@@ -3303,7 +3319,7 @@ def update_monitoring_schedule(
33033319
job_desc = self.sagemaker_session.sagemaker_client.describe_model_quality_job_definition(
33043320
JobDefinitionName=self.job_definition_name
33053321
)
3306-
new_job_definition_name = name_from_base(self.JOB_DEFINITION_BASE_NAME)
3322+
new_job_definition_name = self._generate_job_definition_name()
33073323
request_dict = self._build_create_model_quality_job_definition_request(
33083324
monitoring_schedule_name=self.monitoring_schedule_name,
33093325
job_definition_name=new_job_definition_name,

‎tests/unit/sagemaker/monitor/test_clarify_model_monitor.py‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,21 @@
402402
] = ANALYSIS_CONFIG_LABEL_HEADERS
403403

404404

405+
def _unique_name_from_base(base, *args, **kwargs):
406+
"""Deterministic, collision-free stand-in for name_from_base in tests.
407+
408+
The real name_from_base appends a millisecond timestamp, so two calls in the
409+
same millisecond (create + update within one test) can return identical names.
410+
This counter-based helper keeps each generated name unique and still prefixed
411+
by the base, which is what these tests assert.
412+
"""
413+
_unique_name_from_base.counter += 1
414+
return "{}-{:08d}".format(base, _unique_name_from_base.counter)
415+
416+
417+
_unique_name_from_base.counter = 0
418+
419+
405420
@pytest.fixture()
406421
def sagemaker_client():
407422
return MagicMock()
@@ -905,7 +920,13 @@ def test_model_bias_monitor_creation_failure(model_bias_monitor, sagemaker_sessi
905920
sagemaker_session.sagemaker_client.delete_model_bias_job_definition.assert_called_once()
906921

907922

908-
def test_model_bias_monitor_update_failure(model_bias_monitor, sagemaker_session):
923+
@patch(
924+
"sagemaker.model_monitor.model_monitoring.name_from_base",
925+
side_effect=_unique_name_from_base,
926+
)
927+
def test_model_bias_monitor_update_failure(
928+
mock_name_from_base, model_bias_monitor, sagemaker_session
929+
):
909930
model_bias_monitor.create_monitoring_schedule(
910931
endpoint_input=ENDPOINT_NAME,
911932
ground_truth_input=GROUND_TRUTH_S3_URI,
@@ -1493,8 +1514,12 @@ def test_model_explainability_monitor_creation_failure(
14931514
sagemaker_session.sagemaker_client.delete_model_explainability_job_definition.assert_called_once()
14941515

14951516

1517+
@patch(
1518+
"sagemaker.model_monitor.model_monitoring.name_from_base",
1519+
side_effect=_unique_name_from_base,
1520+
)
14961521
def test_model_explainability_monitor_update_failure(
1497-
model_explainability_monitor, sagemaker_session
1522+
mock_name_from_base, model_explainability_monitor, sagemaker_session
14981523
):
14991524
model_explainability_monitor.create_monitoring_schedule(
15001525
endpoint_input=ENDPOINT_NAME,

‎tests/unit/sagemaker/monitor/test_model_monitoring.py‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,25 @@ def data_quality_monitor(sagemaker_session):
550550
)
551551

552552

553+
def test_generate_job_definition_name_uses_base_job_name(data_quality_monitor):
554+
"""A user-supplied base_job_name is reflected in the job definition name (#4783)."""
555+
name = data_quality_monitor._generate_job_definition_name()
556+
assert name.startswith(BASE_JOB_NAME + "-")
557+
assert DefaultModelMonitor.JOB_DEFINITION_BASE_NAME not in name
558+
559+
560+
def test_generate_job_definition_name_defaults_without_base_job_name(sagemaker_session):
561+
"""Without base_job_name, the job definition name falls back to the type default."""
562+
monitor = DefaultModelMonitor(
563+
role=ROLE,
564+
instance_count=INSTANCE_COUNT,
565+
instance_type=INSTANCE_TYPE,
566+
sagemaker_session=sagemaker_session,
567+
)
568+
name = monitor._generate_job_definition_name()
569+
assert name.startswith(DefaultModelMonitor.JOB_DEFINITION_BASE_NAME)
570+
571+
553572
@pytest.fixture()
554573
def model_monitor_arguments(sagemaker_session):
555574
return ModelMonitor(
@@ -953,7 +972,28 @@ def test_data_quality_monitor_invalid_attach(data_quality_monitor, sagemaker_ses
953972
)
954973

955974

956-
def test_data_quality_monitor_update_failure(data_quality_monitor, sagemaker_session):
975+
def _unique_name_from_base(base, *args, **kwargs):
976+
"""Deterministic, collision-free stand-in for name_from_base in tests.
977+
978+
The real name_from_base appends a millisecond timestamp, so two calls in the
979+
same millisecond (create + update within one test) can return identical names.
980+
This counter-based helper keeps each generated name unique and still prefixed
981+
by the base, which is what these tests assert.
982+
"""
983+
_unique_name_from_base.counter += 1
984+
return "{}-{:08d}".format(base, _unique_name_from_base.counter)
985+
986+
987+
_unique_name_from_base.counter = 0
988+
989+
990+
@patch(
991+
"sagemaker.model_monitor.model_monitoring.name_from_base",
992+
side_effect=_unique_name_from_base,
993+
)
994+
def test_data_quality_monitor_update_failure(
995+
mock_name_from_base, data_quality_monitor, sagemaker_session
996+
):
957997
data_quality_monitor.create_monitoring_schedule(
958998
endpoint_input=ENDPOINT_NAME,
959999
)
@@ -1623,7 +1663,13 @@ def test_model_quality_monitor_invalid_attach(model_quality_monitor, sagemaker_s
16231663
)
16241664

16251665

1626-
def test_model_quality_monitor_update_failure(model_quality_monitor, sagemaker_session):
1666+
@patch(
1667+
"sagemaker.model_monitor.model_monitoring.name_from_base",
1668+
side_effect=_unique_name_from_base,
1669+
)
1670+
def test_model_quality_monitor_update_failure(
1671+
mock_name_from_base, model_quality_monitor, sagemaker_session
1672+
):
16271673
model_quality_monitor.create_monitoring_schedule(
16281674
endpoint_input=ENDPOINT_NAME,
16291675
ground_truth_input=GROUND_TRUTH_S3_URI,

0 commit comments

Comments
 (0)