Skip to content

fix(serve): expose orchestrator deployment fields - #6467

Open
lucasjia-aws wants to merge 1 commit into
aws:masterfrom
lucasjia-aws:fix/6198-modelbuilder-orchestrator-fields
Open

lucasjia-aws wants to merge 1 commit into
aws:masterfrom
lucasjia-aws:fix/6198-modelbuilder-orchestrator-fields

Conversation

@lucasjia-aws

Copy link
Copy Markdown
Collaborator

Summary

ModelBuilder already reads resource_requirements and inference_component_name while building CustomOrchestrator deployables, but neither value is declared as a dataclass constructor field. Documented constructor usage therefore fails with an unexpected keyword argument error.

This change exposes both values as trailing optional ModelBuilder fields, preserving the positional order and defaults of existing constructor arguments. CustomOrchestrator deployables retain both values during build.

Testing

  • Added constructor coverage for both public fields.
  • Added a CustomOrchestrator regression proving the deployable retains the component name and resource requirements.
  • Ran the targeted constructor, build, and existing inference-component workflow tests: 3 passed.
  • Black, Flake8, pydocstyle, signature checks, and git diff --check passed.
  • Built the sagemaker-serve wheel and source distribution successfully.

Fixes #6198

Restore ModelBuilder constructor support for resource requirements and
inference component names, preserving them in custom orchestrator
deployables.

Fixes aws#6198

@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 trailing optional ModelBuilder dataclass fields so documented constructor usage no longer fails with "unexpected keyword argument", and simplifies the now-redundant getattr/hasattr fallbacks that were only there because the attributes weren't declared. The field placement correctly preserves positional ordering of existing constructor args, the removal of the _initialize_jumpstart_config fallback for resource_requirements is safe because the dataclass field now guarantees the attribute, and the direct attribute access in _deploy_core_endpoint is likewise safe. The added tests verify constructor acceptance and that CustomOrchestrator deployables retain both values. The change is correct and adequately tested; no issues found.

Reviewed commit 3dfd6e8. Automated review; verify before acting.

This branch was successfully deployed

1 active deployment
auto-approve — 3dfd6e89 Deployed Oct 13, 2026 by lucasjia-aws via wait-for-approval #1752
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

1 participant