Skip to content

Commit 6bf17a5

Browse files
mohamedzeidan2021Mohamed Zeidan
andauthored
fix: convert ParallelismConfiguration to a dict before passing to boto (#6329)
Pipeline._create_args (used by create/update/upsert) and start() forwarded the raw ParallelismConfiguration object to boto's create/update/start pipeline APIs, which expect a dict ({"MaxParallelExecutionSteps": int}) and reject the object with a ParamValidationError. Add a module-level _resolve_parallelism_config helper that converts the object via to_request(), passes a dict through unchanged (so the pre-fix .to_request() workaround keeps working), and returns None as-is so update_args drops the key. Wrap both boto entry points with it. Adds regression tests (object->dict at create and start), an offline boto ParamValidator end-to-end proof, and a dict/None passthrough test. Fixes #5354 Co-authored-by: Mohamed Zeidan <zeidmo@amazon.com>
1 parent f2c42b4 commit 6bf17a5

2 files changed

Lines changed: 116 additions & 2 deletions

File tree

‎src/sagemaker/workflow/pipeline.py‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,10 @@ def _create_args(
238238
}
239239

240240
update_args(
241-
kwargs, PipelineDescription=description, ParallelismConfiguration=parallelism_config
241+
kwargs,
242+
PipelineDescription=description,
243+
# boto expects a dict, not a ParallelismConfiguration object (see issue #5354).
244+
ParallelismConfiguration=_resolve_parallelism_config(parallelism_config),
242245
)
243246
return kwargs
244247

@@ -403,7 +406,8 @@ def start(
403406
kwargs,
404407
PipelineExecutionDescription=execution_description,
405408
PipelineExecutionDisplayName=execution_display_name,
406-
ParallelismConfiguration=parallelism_config,
409+
# boto expects a dict, not a ParallelismConfiguration object (see issue #5354).
410+
ParallelismConfiguration=_resolve_parallelism_config(parallelism_config),
407411
SelectiveExecutionConfig=selective_execution_config,
408412
PipelineVersionId=pipeline_version_id,
409413
)
@@ -862,6 +866,20 @@ def _map_lambda_outputs(steps: List[Step]):
862866
return lambda_output_map
863867

864868

869+
def _resolve_parallelism_config(parallelism_config):
870+
"""Normalize a parallelism_config into the request dict boto expects.
871+
872+
boto's create/update/start pipeline APIs expect ``ParallelismConfiguration`` as a dict
873+
(``{"MaxParallelExecutionSteps": int}``), not a ``ParallelismConfiguration`` object
874+
(issue #5354). This converts the object via ``to_request()``. A dict is passed through
875+
unchanged so callers who adopted the pre-fix ``.to_request()`` workaround keep working,
876+
and ``None`` is returned as-is so ``update_args`` can drop the key.
877+
"""
878+
if isinstance(parallelism_config, ParallelismConfiguration):
879+
return parallelism_config.to_request()
880+
return parallelism_config
881+
882+
865883
def update_args(args: Dict[str, Any], **kwargs):
866884
"""Updates the request arguments dict with a value, if populated.
867885

‎tests/unit/sagemaker/workflow/test_pipeline.py‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,102 @@ def test_pipeline_create_and_start_with_parallelism_config(sagemaker_session_moc
174174
)
175175

176176

177+
def test_pipeline_create_converts_parallelism_config_object_to_dict(
178+
sagemaker_session_mock, role_arn
179+
):
180+
"""Regression for #5354.
181+
182+
create() (via _create_args) must convert a ParallelismConfiguration object to its request
183+
dict, because boto rejects the object with a ParamValidationError.
184+
"""
185+
from sagemaker.workflow.parallelism_config import ParallelismConfiguration
186+
187+
pipeline = Pipeline(
188+
name="MyPipeline",
189+
parameters=[],
190+
steps=[],
191+
sagemaker_session=sagemaker_session_mock,
192+
)
193+
pipeline.create(
194+
role_arn=role_arn,
195+
parallelism_config=ParallelismConfiguration(max_parallel_execution_steps=5),
196+
)
197+
call_kwargs = sagemaker_session_mock.sagemaker_client.create_pipeline.call_args[1]
198+
assert call_kwargs["ParallelismConfiguration"] == {"MaxParallelExecutionSteps": 5}
199+
assert isinstance(call_kwargs["ParallelismConfiguration"], dict)
200+
201+
202+
def test_pipeline_start_converts_parallelism_config_object_to_dict(
203+
sagemaker_session_mock, role_arn
204+
):
205+
"""Regression for #5354: start() must pass a dict to boto, not a ParallelismConfiguration."""
206+
from sagemaker.workflow.parallelism_config import ParallelismConfiguration
207+
208+
sagemaker_session_mock.sagemaker_client.start_pipeline_execution.return_value = dict(
209+
PipelineExecutionArn="pipeline-execution-arn"
210+
)
211+
pipeline = Pipeline(
212+
name="MyPipeline",
213+
parameters=[],
214+
steps=[],
215+
sagemaker_session=sagemaker_session_mock,
216+
)
217+
pipeline.start(parallelism_config=ParallelismConfiguration(max_parallel_execution_steps=7))
218+
call_kwargs = sagemaker_session_mock.sagemaker_client.start_pipeline_execution.call_args[1]
219+
assert call_kwargs["ParallelismConfiguration"] == {"MaxParallelExecutionSteps": 7}
220+
assert isinstance(call_kwargs["ParallelismConfiguration"], dict)
221+
222+
223+
def test_resolve_parallelism_config_output_passes_real_boto_param_validation():
224+
"""End-to-end proof for #5354 against the real SageMaker service model (offline).
225+
226+
boto3's client-side ParamValidator (no network, no credentials) is exactly what raised the
227+
reported ParamValidationError. This asserts the raw ParallelismConfiguration object is
228+
rejected by that validator, while the value produced by ``_resolve_parallelism_config`` is
229+
accepted -- reproducing the bug and proving the fix at the boto layer, not just via mocks.
230+
"""
231+
import boto3
232+
from botocore import validate
233+
234+
from sagemaker.workflow.parallelism_config import ParallelismConfiguration
235+
from sagemaker.workflow.pipeline import _resolve_parallelism_config
236+
237+
shape = (
238+
boto3.client("sagemaker", region_name="us-west-2")
239+
.meta.service_model.operation_model("CreatePipeline")
240+
.input_shape
241+
)
242+
validator = validate.ParamValidator()
243+
base = {
244+
"PipelineName": "p",
245+
"RoleArn": "arn:aws:iam::111111111111:role/x",
246+
"PipelineDefinition": "{}",
247+
"ClientRequestToken": "x" * 32,
248+
}
249+
cfg = ParallelismConfiguration(max_parallel_execution_steps=5)
250+
251+
# The raw object is what the old code passed -- boto rejects it.
252+
raw_report = validator.validate({**base, "ParallelismConfiguration": cfg}, shape)
253+
assert raw_report.has_errors()
254+
255+
# The converted value the fix passes -- boto accepts it.
256+
fixed_report = validator.validate(
257+
{**base, "ParallelismConfiguration": _resolve_parallelism_config(cfg)}, shape
258+
)
259+
assert not fixed_report.has_errors()
260+
261+
262+
def test_resolve_parallelism_config_passes_through_dict_and_none():
263+
"""Backwards compat for #5354: a dict (pre-fix workaround) is forwarded unchanged and
264+
None is returned as-is so update_args drops the key."""
265+
from sagemaker.workflow.pipeline import _resolve_parallelism_config
266+
267+
assert _resolve_parallelism_config({"MaxParallelExecutionSteps": 3}) == {
268+
"MaxParallelExecutionSteps": 3
269+
}
270+
assert _resolve_parallelism_config(None) is None
271+
272+
177273
@patch("sagemaker.s3.S3Uploader.upload_string_as_file_body")
178274
def test_large_pipeline_create(sagemaker_session_mock, role_arn):
179275
sagemaker_session_mock.sagemaker_config = {}

0 commit comments

Comments
 (0)