Skip to content

Commit 9e2e863

Browse files
author
Mohamed Zeidan
committed
fix: keep job name in pipeline request for ModelTrainer and HyperparameterTuner
Both unconditionally popped training_job_name / hyper_parameter_tuning_job_name from the request for a PipelineSession. The TrainingStep/TuningStep already strip it via trim_request_dict (dropped by default, prefix preserved when PipelineDefinitionConfig(use_custom_job_prefix=True)), so popping upstream left use_custom_job_prefix nothing to preserve and made base_job_name have no effect in pipelines. Stop popping; let trim_request_dict handle it. Fixes #5776, #6299
1 parent 7ac2708 commit 9e2e863

4 files changed

Lines changed: 109 additions & 11 deletions

File tree

‎sagemaker-train/src/sagemaker/train/model_trainer.py‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
from sagemaker.core.shapes import AlgorithmSpecification, ModelPackageConfig
3333
from sagemaker.core.utils.utils import serialize
3434
from sagemaker.core.apiutils._boto_functions import to_pascal_case
35+
from sagemaker.core.common_utils import name_from_base
3536

3637
from pydantic import BaseModel, ConfigDict, PrivateAttr, validate_call
3738
from sagemaker.core.config.config_schema import (
@@ -75,7 +76,6 @@
7576
from sagemaker.train.distributed import Torchrun, DistributedConfig
7677
from sagemaker.train.utils import (
7778
_default_s3_uri,
78-
_get_unique_name,
7979
_is_valid_path,
8080
_is_valid_s3_uri,
8181
safe_serialize,
@@ -619,7 +619,13 @@ def _create_training_job_args(
619619
Dict[str, Any]: The training job arguments.
620620
"""
621621
self._populate_intelligent_defaults()
622-
current_training_job_name = _get_unique_name(self.base_job_name)
622+
# Use the shared name_from_base timestamp format so that
623+
# sagemaker.core.common_utils.base_from_name can strip it back to base_job_name.
624+
# trim_request_dict relies on base_from_name to preserve the prefix when a pipeline
625+
# sets PipelineDefinitionConfig(use_custom_job_prefix=True) (issues #5776, #6299).
626+
# Underscores are replaced because SageMaker job names disallow them; _get_unique_name
627+
# used to do this and name_from_base does not.
628+
current_training_job_name = name_from_base(self.base_job_name.replace("_", "-"))
623629
input_data_key_prefix = f"{self.base_job_name}/{current_training_job_name}/input"
624630

625631
final_input_data_config = self.input_data_config.copy() if self.input_data_config else []
@@ -814,8 +820,10 @@ def _create_training_job_args(
814820
training_request["model_package_config"] = self.model_package_config
815821

816822
if boto3 or isinstance(self.sagemaker_session, PipelineSession):
817-
if isinstance(self.sagemaker_session, PipelineSession):
818-
training_request.pop("training_job_name", None)
823+
# Keep training_job_name in the request. The TrainingStep strips it via
824+
# trim_request_dict, which drops it by default but preserves the base_job_name
825+
# prefix when PipelineDefinitionConfig(use_custom_job_prefix=True). Popping it here
826+
# unconditionally left use_custom_job_prefix nothing to preserve (issue #5776, #6299).
819827
# Convert snake_case to PascalCase for AWS API
820828
pipeline_request = {to_pascal_case(k): v for k, v in training_request.items()}
821829
serialized_request = serialize(pipeline_request)

‎sagemaker-train/src/sagemaker/train/tuner.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,8 +1290,10 @@ def _start_tuning_job(self, inputs):
12901290
from sagemaker.core.utils.utils import serialize
12911291
from sagemaker.core.apiutils._boto_functions import to_pascal_case
12921292

1293-
# Remove job name for pipeline as it's auto-generated at execution time
1294-
tuning_request.pop("hyper_parameter_tuning_job_name", None)
1293+
# Keep hyper_parameter_tuning_job_name in the request. The TuningStep strips it via
1294+
# trim_request_dict, which drops it by default but preserves the base_tuning_job_name
1295+
# prefix when PipelineDefinitionConfig(use_custom_job_prefix=True). Popping it here
1296+
# unconditionally left use_custom_job_prefix nothing to preserve (issue #6299, #5776).
12951297
# Convert snake_case to PascalCase for AWS API
12961298
pipeline_request = {to_pascal_case(k): v for k, v in tuning_request.items()}
12971299
serialized_request = serialize(pipeline_request)

‎sagemaker-train/tests/unit/train/test_model_trainer.py‎

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,60 @@ def test_model_trainer_param_validation(test_case, modules_session):
254254
assert trainer.base_job_name == DEFAULT_BASE_NAME
255255

256256

257+
def test_pipeline_session_request_keeps_training_job_name(modules_session):
258+
"""Regression for #5776 / #6299.
259+
260+
Under a PipelineSession the request must KEEP TrainingJobName. The TrainingStep strips it via
261+
trim_request_dict (dropped by default, prefix preserved when use_custom_job_prefix=True);
262+
popping it here left use_custom_job_prefix nothing to preserve.
263+
"""
264+
from unittest.mock import Mock
265+
from sagemaker.core.workflow.pipeline_context import PipelineSession
266+
267+
session = Mock(spec=PipelineSession)
268+
session.default_bucket.return_value = DEFAULT_BUCKET
269+
session.default_bucket_prefix = DEFAULT_BUCKET_PREFIX
270+
session.boto_region_name = DEFAULT_REGION
271+
session.sagemaker_config = {}
272+
273+
trainer = ModelTrainer(
274+
training_image=DEFAULT_IMAGE,
275+
role=DEFAULT_ROLE,
276+
compute=DEFAULT_COMPUTE_CONFIG,
277+
stopping_condition=DEFAULT_STOPPING_CONDITION,
278+
output_data_config=DEFAULT_OUTPUT_DATA_CONFIG,
279+
base_job_name="my-prefix",
280+
sagemaker_session=session,
281+
)
282+
283+
args = trainer._create_training_job_args(input_data_config=[])
284+
285+
assert "TrainingJobName" in args, "TrainingJobName must survive into the pipeline request"
286+
assert args["TrainingJobName"].startswith("my-prefix")
287+
288+
# The generated name must use a timestamp format base_from_name can strip, otherwise
289+
# trim_request_dict bakes a definition-time timestamp into the pipeline definition
290+
# instead of preserving the base prefix.
291+
from sagemaker.core.common_utils import base_from_name
292+
293+
assert base_from_name(args["TrainingJobName"]) == "my-prefix"
294+
295+
# End-to-end: with use_custom_job_prefix=True, trim_request_dict must leave exactly the base.
296+
from types import SimpleNamespace
297+
from sagemaker.core.workflow.utilities import trim_request_dict
298+
from sagemaker.core.workflow.pipeline_definition_config import PipelineDefinitionConfig
299+
300+
custom_prefix_config = SimpleNamespace(
301+
pipeline_definition_config=PipelineDefinitionConfig(use_custom_job_prefix=True)
302+
)
303+
trimmed = trim_request_dict(dict(args), "TrainingJobName", custom_prefix_config)
304+
assert trimmed["TrainingJobName"] == "my-prefix"
305+
306+
# And by default (no config) the key is dropped entirely.
307+
dropped = trim_request_dict(dict(args), "TrainingJobName", None)
308+
assert "TrainingJobName" not in dropped
309+
310+
257311
@patch("sagemaker.train.model_trainer.TrainingJob")
258312
def test_train_with_default_params(mock_training_job, model_trainer):
259313
model_trainer.train()
@@ -871,7 +925,7 @@ def test_remote_debug_config(mock_training_job, modules_session):
871925
)
872926

873927

874-
@patch("sagemaker.train.model_trainer._get_unique_name")
928+
@patch("sagemaker.train.model_trainer.name_from_base")
875929
@patch("sagemaker.train.model_trainer.TrainingJob")
876930
def test_model_trainer_full_init(mock_training_job, mock_unique_name, modules_session):
877931
def mock_upload_data(path, bucket, key_prefix):
@@ -1147,7 +1201,7 @@ def mock_upload_data(path, bucket, key_prefix):
11471201

11481202
# TODO: Re-Enable after local mode fully migrated to v3
11491203
# @patch("sagemaker.train.model_trainer._LocalContainer")
1150-
# @patch("sagemaker.train.model_trainer._get_unique_name")
1204+
# @patch("sagemaker.train.model_trainer.name_from_base")
11511205
# @patch("sagemaker.train.local.local_container.download_folder")
11521206
# def test_model_trainer_local_full_init(
11531207
# mock_download_folder, mock_unique_name, mock_local_container, modules_session
@@ -1403,7 +1457,7 @@ def test_hyperparameters_invalid(mock_exists, modules_session):
14031457
)
14041458

14051459

1406-
@patch("sagemaker.train.model_trainer._get_unique_name")
1460+
@patch("sagemaker.train.model_trainer.name_from_base")
14071461
@patch("sagemaker.train.model_trainer.TrainingJob")
14081462
def test_model_trainer_default_paths(mock_training_job, mock_unique_name, modules_session):
14091463
def mock_upload_data(path, bucket, key_prefix):
@@ -1507,7 +1561,7 @@ def test_metric_definitions(mock_training_job, modules_session):
15071561
)
15081562

15091563

1510-
@patch("sagemaker.train.model_trainer._get_unique_name")
1564+
@patch("sagemaker.train.model_trainer.name_from_base")
15111565
@patch("sagemaker.core.resources.TrainingJob")
15121566
def test_nova_recipe(mock_training_job, mock_unique_name, modules_session):
15131567
def mock_upload_data(path, bucket, key_prefix):
@@ -1779,7 +1833,7 @@ def test_nova_recipe_model_package_config_only_mpg_from_recipe(modules_session):
17791833
os.unlink(recipe.name)
17801834

17811835

1782-
@patch("sagemaker.train.model_trainer._get_unique_name")
1836+
@patch("sagemaker.train.model_trainer.name_from_base")
17831837
@patch("sagemaker.train.model_trainer.TrainingJob")
17841838
def test_llmft_recipe(mock_training_job, mock_unique_name, modules_session):
17851839
def mock_upload_data(path, bucket, key_prefix):

‎sagemaker-train/tests/unit/train/test_tuner.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,40 @@ def test_build_training_job_definition_includes_spot_params(self):
598598
definition.stopping_condition.max_wait_time_in_seconds, int
599599
), "Max wait time should be set"
600600

601+
def test_pipeline_session_request_keeps_tuning_job_name(self):
602+
"""Regression for #6299 / #5776.
603+
604+
Under a PipelineSession the tuning request must KEEP HyperParameterTuningJobName. The
605+
TuningStep strips it via trim_request_dict (dropped by default, prefix preserved when
606+
use_custom_job_prefix=True); popping it here left use_custom_job_prefix nothing to preserve.
607+
"""
608+
from unittest.mock import Mock
609+
from sagemaker.core.workflow.pipeline_context import PipelineSession
610+
611+
pipeline_session = Mock(spec=PipelineSession)
612+
mock_trainer = _create_mock_model_trainer()
613+
mock_trainer.sagemaker_session = pipeline_session
614+
615+
tuner = HyperparameterTuner(
616+
model_trainer=mock_trainer,
617+
objective_metric_name="accuracy",
618+
hyperparameter_ranges=_create_single_hp_range(),
619+
)
620+
tuner._current_job_name = "my-prefix-2026-01-01-00-00-00-000"
621+
622+
tuner._start_tuning_job(inputs=None)
623+
624+
pipeline_session._intercept_create_request.assert_called_once()
625+
serialized_request = pipeline_session._intercept_create_request.call_args.args[0]
626+
assert "HyperParameterTuningJobName" in serialized_request
627+
assert serialized_request["HyperParameterTuningJobName"].startswith("my-prefix")
628+
629+
# The name must be strippable back to the base, or trim_request_dict would preserve a
630+
# definition-time timestamp instead of the prefix when use_custom_job_prefix=True.
631+
from sagemaker.core.common_utils import base_from_name
632+
633+
assert base_from_name(serialized_request["HyperParameterTuningJobName"]) == "my-prefix"
634+
601635
def test_build_training_job_definition_includes_environment_variables(self):
602636
"""Test that _build_training_job_definition includes environment variables.
603637

0 commit comments

Comments
 (0)