feat(BA-6644): Add domain id, resource group id in session row#12452
Conversation
There was a problem hiding this comment.
Pull request overview
This PR denormalizes ownership/placement identifiers onto session rows by adding nullable domain_id and resource_group_id columns to the sessions table, backfilling existing rows, and propagating these IDs through every session-enqueue path (interactive sessions, registry, model-serving dry-run, and deployment enqueue). It also extends the v2 GraphQL/REST EnqueueSessionInput with a new resource_group_id field while deprecating the name-based resource_group, and changes pick_default_resource_group to return a combined id+name identifier.
Changes:
- Add
domain_id/resource_group_idcolumns + Alembic migration (ada41cb881bb) with backfill and indexes. - Resolve and populate domain/resource-group IDs in the session, registry, model-serving, and deployment scope-building flows; introduce
ResourceGroupIdentifierand new id-by-name repository lookups. - Add a
resource_group_idrequest field (GQL/REST DTO) and deprecateresource_group, with schema docs updated.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
models/alembic/versions/ada41cb881bb_add_session_scope_id_columns.py |
New migration adding/backfilling the two ID columns and indexes |
models/session/row.py |
Adds domain_id/resource_group_id ORM columns (uses bare GUID for typed columns) |
services/session/service.py |
Resolves domain/RG IDs during enqueue; RG selection still gated on the deprecated name |
services/session/actions/enqueue_session.py |
Adds resource_group_id to the resource spec |
services/model_serving/services/model_serving.py |
Builds session scope with resolved domain/RG IDs |
registry.py |
Builds session scope with resolved domain/RG IDs |
repositories/scheduler/repository.py / db_source/db_source.py |
New get_domain_id_by_name/get_resource_group_id_by_name; pick_default_resource_group returns identifier |
repositories/scheduler/types/session_creation.py |
New ResourceGroupIdentifier; AllowedScalingGroup gains typed id/name |
repositories/deployment/db_source/db_source.py |
Resolves and threads domain/RG IDs into DeploymentContext |
repositories/scheduler/creators.py |
Persists domain_id/resource_group_id on the row |
data/session/spec.py, draft.py, creation.py |
Adds id fields to scope/draft/deployment-context models |
sokovan/deployment/deployment_draft_builder.py |
Passes context IDs into the session scope draft |
api/gql/session/types.py, api/adapters/session/adapter.py, dto/.../session/request.py |
Adds resource_group_id input field, deprecates resource_group, maps it into the action |
docs/manager/graphql-reference/*.graphql |
Schema docs for the new/deprecated input fields |
tests/..., changes/12452.feature.md |
Updates fixtures/assertions for new repos and identifier types; changelog |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self, | ||
| db_sess: SASession, | ||
| deployment_info: DeploymentInfo, | ||
| ) -> tuple[DomainID, ResourceGroupID]: |
There was a problem hiding this comment.
I wrote it this way because this method is used in only one place, and since it will be removed during future refactoring based on the ID, I didn't wrap it in a separate data class or split it into two methods.
| sessions: Mapped[list[SessionRow]] = relationship( | ||
| "SessionRow", | ||
| back_populates="domain", | ||
| foreign_keys="[SessionRow.domain_name]", | ||
| ) |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
Test plan
Resolves BA-6644
📚 Documentation preview 📚: https://sorna--12452.org.readthedocs.build/en/12452/
📚 Documentation preview 📚: https://sorna-ko--12452.org.readthedocs.build/ko/12452/