Skip to content

Commit b9a0ea7

Browse files
fix(serve): pass instance_type to _deploy once when creating an inference component (#6203)
Deploying a CustomOrchestrator in InferenceComponent mode through ModelBuilder.deploy() always raised TypeError: _deploy() got multiple values for keyword argument 'instance_type' In _deploy_for_ic the branch that creates a new inference component read instance_type and initial_instance_count out of kwargs with kwargs.get, passed them to self._deploy as explicit keyword arguments, and then spread the unchanged **kwargs into the same call. kwargs.get leaves the key in place, so Python saw each of those arguments twice. The only caller of this branch, ModelBuilder.deploy, always supplies both keys, so the failure was deterministic. Consume the two keys with kwargs.pop instead so each is forwarded exactly once. The defaults (self.instance_type and 1) are unchanged and kwargs is a local dict, so the caller is unaffected. _deploy_core_endpoint reads the values back through kwargs.get and receives the same data as before. Add unit tests covering the create branch of _deploy_for_ic with explicit instance arguments (fails with the TypeError before this change) and with the default fallbacks. Fixes #6199 Co-authored-by: lucasjia-aws <lucasjia@amazon.com>
1 parent 1f09f26 commit b9a0ea7

2 files changed

Lines changed: 70 additions & 2 deletions

File tree

‎sagemaker-serve/src/sagemaker/serve/model_builder.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4355,8 +4355,8 @@ def _deploy_for_ic(self, ic_data: Dict[str, Any], endpoint_name: str, **kwargs)
43554355
endpoint_type=EndpointType.INFERENCE_COMPONENT_BASED,
43564356
resources=resource_requirements,
43574357
inference_component_name=ic_name,
4358-
instance_type=kwargs.get("instance_type", self.instance_type),
4359-
initial_instance_count=kwargs.get("initial_instance_count", 1),
4358+
instance_type=kwargs.pop("instance_type", self.instance_type),
4359+
initial_instance_count=kwargs.pop("initial_instance_count", 1),
43604360
**kwargs,
43614361
)
43624362

‎sagemaker-serve/tests/unit/test_model_builder_deploy.py‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,74 @@ def test_does_ic_exist_false(self):
514514

515515
self.assertFalse(result)
516516

517+
def test_deploy_for_ic_creates_new_ic_with_explicit_instance_kwargs(self):
518+
"""Test _deploy_for_ic forwards instance_type and initial_instance_count to _deploy once."""
519+
builder = ModelBuilder(
520+
model=Mock(),
521+
role_arn="arn:aws:iam::123456789012:role/TestRole",
522+
sagemaker_session=self.mock_session,
523+
)
524+
builder._does_ic_exist = Mock(return_value=False)
525+
builder._deploy = Mock(return_value="endpoint")
526+
built_model = Mock()
527+
resource_requirements = ResourceRequirements(requests={"memory": 1024, "copies": 1})
528+
ic_data = {
529+
"Name": "test-ic",
530+
"ResourceRequirements": resource_requirements,
531+
"Model": built_model,
532+
}
533+
534+
result = builder._deploy_for_ic(
535+
ic_data=ic_data,
536+
endpoint_name="test-endpoint",
537+
container_timeout_in_seconds=600,
538+
instance_type="ml.g5.xlarge",
539+
initial_instance_count=2,
540+
)
541+
542+
self.assertEqual(result, "endpoint")
543+
builder._deploy.assert_called_once_with(
544+
built_model=built_model,
545+
endpoint_name="test-endpoint",
546+
endpoint_type=EndpointType.INFERENCE_COMPONENT_BASED,
547+
resources=resource_requirements,
548+
inference_component_name="test-ic",
549+
instance_type="ml.g5.xlarge",
550+
initial_instance_count=2,
551+
container_timeout_in_seconds=600,
552+
)
553+
554+
def test_deploy_for_ic_creates_new_ic_with_default_instance_kwargs(self):
555+
"""Test _deploy_for_ic falls back to builder instance_type and one instance."""
556+
builder = ModelBuilder(
557+
model=Mock(),
558+
role_arn="arn:aws:iam::123456789012:role/TestRole",
559+
sagemaker_session=self.mock_session,
560+
)
561+
builder.instance_type = "ml.c5.xlarge"
562+
builder._does_ic_exist = Mock(return_value=False)
563+
builder._deploy = Mock(return_value="endpoint")
564+
built_model = Mock()
565+
resource_requirements = ResourceRequirements(requests={"memory": 1024, "copies": 1})
566+
ic_data = {
567+
"Name": "test-ic",
568+
"ResourceRequirements": resource_requirements,
569+
"Model": built_model,
570+
}
571+
572+
result = builder._deploy_for_ic(ic_data=ic_data, endpoint_name="test-endpoint")
573+
574+
self.assertEqual(result, "endpoint")
575+
builder._deploy.assert_called_once_with(
576+
built_model=built_model,
577+
endpoint_name="test-endpoint",
578+
endpoint_type=EndpointType.INFERENCE_COMPONENT_BASED,
579+
resources=resource_requirements,
580+
inference_component_name="test-ic",
581+
instance_type="ml.c5.xlarge",
582+
initial_instance_count=1,
583+
)
584+
517585

518586
class TestModelBuilderResetState(unittest.TestCase):
519587
"""Test ModelBuilder _reset_build_state method."""

0 commit comments

Comments
 (0)