Skip to content

fix: expose inference component fields on ModelBuilder - #6457

Open
gauravkriplani wants to merge 2 commits into
aws:masterfrom
gauravkriplani:fix/modelbuilder-custom-orchestrator-fields
Open

gauravkriplani wants to merge 2 commits into
aws:masterfrom
gauravkriplani:fix/modelbuilder-custom-orchestrator-fields

Conversation

@gauravkriplani

Copy link
Copy Markdown

Summary

  • expose resource_requirements and inference_component_name as typed ModelBuilder fields
  • allow documented CustomOrchestrator inference-component configurations to be passed through the constructor
  • verify both values propagate into the generated inference-component deployable

Problem

ModelBuilder.build() consumes these attributes internally, but the dataclass constructor did not declare them. Documented configurations therefore failed with TypeError and required users to mutate the builder after construction.

Testing

  • pytest -q sagemaker-serve/tests/unit/test_model_builder_workflows.py — 17 passed
  • exact CustomOrchestrator constructor reproduction
  • black --check
  • git diff --check

Fixes #6198

@gauravkriplani
gauravkriplani deployed to manual-approval October 13, 2026 18:12 — with GitHub Actions Active
@gauravkriplani
gauravkriplani deployed to manual-approval October 13, 2026 18:12 — with GitHub Actions Active
@gauravkriplani
gauravkriplani deployed to manual-approval October 13, 2026 18:12 — with GitHub Actions Active

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI code review

This PR exposes resource_requirements and inference_component_name as typed ModelBuilder dataclass fields (both defaulting to None), and removes the now-redundant hasattr guard for resource_requirements in _initialize_jumpstart_config. These attributes were already consumed by build() (e.g. lines 4649-4650, 4711-4713), so declaring them as constructor fields matches the documented CustomOrchestrator API and prevents TypeError. The change is correct and well-scoped: imports (field, Optional, ResourceRequirements) are all present, defaults are safe, and the updated test verifies propagation into the generated inference-component deployable. No issues found.

Reviewed commit eaa31d9. Automated review; verify before acting.

This branch is waiting to be deployed

1 waiting deployment
manual-approval — b299c86b Waiting Nov 3, 2026 by mufaddal-rohawala via wait-for-approval #798
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ModelBuilder missing resource_requirements and inference_component_name fields for CustomOrchestrator IC deployment

2 participants