Skip to content

feat(BA-6006): Add resource slot validation when creating deployment revision#11580

Merged
HyeockJinKim merged 5 commits into
mainfrom
feat/BA-6006
May 14, 2026
Merged

feat(BA-6006): Add resource slot validation when creating deployment revision#11580
HyeockJinKim merged 5 commits into
mainfrom
feat/BA-6006

Conversation

@seedspirit
Copy link
Copy Markdown
Contributor

Summary

  • Add DeploymentRevisionValidator chain (sokovan/deployment/validators/) with RequiredResourceSlotRule mirroring PR feat(BA-5999): Validate session spec includes required resource slots in session creation #11556's session-side rule, so deployment revision creation fails fast with InvalidAPIParameters when the finalized revision spec omits a globally required resource slot.
  • Two validator entry points handle the different finalized shapes per path: validate(spec: DeploymentRevisionCreatorSpec, ctx) for v2 add_revision, and validate_legacy_revision_spec(spec: ModelRevisionSpec, ctx) for build_creator_from_legacy_draft / resolve_legacy_revision_spec. Both entries are abstract on the rule base class so new rules must wire both paths.
  • New fetch_revision_required_slot_names on DeploymentRepository / DeploymentDBSource reads resource_slot_types.required; the controller assembles DeploymentRevisionValidationContext once per call site.
  • Intentional dual-layer defense: this PR adds revision-creation-time enforcement (prevents dead-revision / orphan-endpoint cases). Session enqueue-time enforcement via scheduling_controller._spec_validator (added in feat(BA-5999): Validate session spec includes required resource slots in session creation #11556) stays in place to cover non-revision session paths.

Test plan

  • Unit tests in tests/unit/manager/sokovan/deployment/test_revision_validators.py pass.
  • v2 add_revision: 4xx returned and no row written when a required slot is missing.
  • Legacy create flow (build_creator_from_legacy_draft): 4xx returned before endpoint creation when a required slot is missing.
  • resolve_legacy_revision_spec: 4xx returned before returning the spec when a required slot is missing.
  • Normal request with all required slots present succeeds end-to-end on all three paths.

Resolves BA-6006

@github-actions github-actions Bot added the size:L 100~500 LoC label May 13, 2026
@github-actions github-actions Bot added the comp:manager Related to Manager component label May 13, 2026
@seedspirit seedspirit marked this pull request as ready for review May 13, 2026 07:27
@seedspirit seedspirit requested a review from a team as a code owner May 13, 2026 07:27
Copilot AI review requested due to automatic review settings May 13, 2026 07:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an explicit deployment-revision validation layer so revision creation fails fast when required resource slots (from resource_slot_types.required) are missing or zero, preventing “dead revisions” and related orphan states before persistence.

Changes:

  • Introduces a deployment revision validator framework (DeploymentRevisionValidator + rule base + context) with dual entry points for v2 vs legacy spec shapes.
  • Adds RequiredResourceSlotRule and wires it into deployment revision creation flows (add_revision, build_creator_from_legacy_draft, resolve_legacy_revision_spec).
  • Adds repository/DBSource support to fetch globally required slot names and includes unit tests + changelog entry.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/unit/manager/sokovan/deployment/test_revision_validators.py Adds unit tests covering required-slot enforcement for both v2 and legacy validator entry points.
src/ai/backend/manager/sokovan/deployment/validators/required_resource_slot_rule.py Implements the required resource-slot rule for deployment revision specs.
src/ai/backend/manager/sokovan/deployment/validators/deployment_revision_base.py Adds the validator chain, rule interface, and validation context for deployment revisions.
src/ai/backend/manager/sokovan/deployment/validators/__init__.py Exposes the new validator components via package exports.
src/ai/backend/manager/sokovan/deployment/deployment_controller.py Wires revision validation into v2 and legacy revision creation/resolve paths; builds validation context per call site.
src/ai/backend/manager/repositories/deployment/repository.py Adds repository method to fetch globally required slot names for revision validation.
src/ai/backend/manager/repositories/deployment/db_source/db_source.py Implements DB query to retrieve required slot names from resource_slot_types.required.
changes/11580.feature.md Adds release note entry for the new revision creation validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

context: DeploymentRevisionValidationContext,
) -> None:
self._check_resource_slots(
ResourceSlot(spec.resource_spec.resource_slots),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would like the type conversion to occur at the time of the initial input.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since the type of resource_slots in the existing ResourceSpec is Mapping[str, Any], modifying this part as well would likely make the scope too broad.

jopemachine
jopemachine previously approved these changes May 13, 2026
Replace concrete frozenset[]/tuple[] type annotations on the revision
validator chain with abstract Iterable/Sequence so callers are not
locked into specific concrete containers. Runtime values stay frozenset
for the required slot names and tuple for the rule chain.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@HyeockJinKim HyeockJinKim merged commit c3347d3 into main May 14, 2026
33 checks passed
@HyeockJinKim HyeockJinKim deleted the feat/BA-6006 branch May 14, 2026 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:manager Related to Manager component size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants