refactor(BA-5711): rename SessionData user_uuid to owner_id#11045
Closed
jopemachine wants to merge 6 commits into
Closed
refactor(BA-5711): rename SessionData user_uuid to owner_id#11045jopemachine wants to merge 6 commits into
jopemachine wants to merge 6 commits into
Conversation
This was referenced Apr 14, 2026
This was referenced Apr 14, 2026
This was referenced Apr 14, 2026
6d34e21 to
d88a14b
Compare
0b27565 to
caa8001
Compare
jopemachine
commented
Apr 14, 2026
jopemachine
commented
Apr 14, 2026
caa8001 to
7bac45f
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors session data ownership identifiers by renaming SessionData.user_uuid / SessionMetadata.user_uuid to owner_id and removing redundant access-key snapshot fields, while updating adapters and query helpers to use owner_id and resolve main_access_key from the owning user.
Changes:
- Rename session dataclass fields to
owner_idand dropaccess_keysnapshot fields. - Update
SessionRowadapters (to_dataclass/from_dataclass,to_session_info/from_session_info) and session fetch/match helpers to scope byowner_id. - Update legacy GraphQL session node building to resolve
main_access_key(async fallback) and update resource-usage computation to source access key from the user relationship.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/ai/backend/manager/models/session/row.py |
Refactors session lookup helpers to owner_id, updates dataclass/session-info adapters, removes keypair relationship, and renames a session-name uniqueness index. |
src/ai/backend/manager/models/resource_usage.py |
Switches resource-usage access-key sourcing to user.main_access_key and removes SessionRow.access_key from selected columns. |
src/ai/backend/manager/data/session/types.py |
Renames user_uuid → owner_id and removes access-key snapshot fields from session dataclasses. |
src/ai/backend/manager/api/gql_legacy/session.py |
Makes ComputeSessionNode.from_dataclass async and resolves owner main_access_key via eager owner data or UserRepository. |
changes/11045.misc.md |
Adds a changelog entry describing the rename and access-key snapshot removal. |
Comments suppressed due to low confidence (1)
src/ai/backend/manager/models/session/row.py:863
- Renaming the SQLAlchemy index to
ix_sessions_unique_name_per_owner_nonterminalwithout a matching Alembic migration will make schema comparisons/autogenerate treat this as a drop/create (the existing migration createsix_sessions_unique_name_per_user_nonterminal; seemodels/alembic/versions/a4289ef5f0cd_allow_duplicated_session_names_across_.py). Either keep the existing index name for compatibility or add a migration that renames/drops+creates the index with the new name.
sa.Index(
"ix_sessions_unique_name_per_owner_nonterminal",
"name",
"user_uuid",
unique=True,
postgresql_where=sa.text("status NOT IN ('ERROR', 'TERMINATED', 'CANCELLED')"),
),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5ea1195 to
cfdf874
Compare
Data-layer rename of SessionData / SessionMetadata user_uuid to owner_id, plus the matching SessionRow adapters (to_dataclass/from_dataclass/to_session_info/from_session_info). ComputeSessionNode.from_dataclass becomes async and resolves main_access_key from the owner via UserRepository when owner is not eagerly loaded. models/resource_usage.py sources user_id from SessionRow.user_uuid through the session relationship. SessionRow._build_session_fetch_query / _match_sessions_by_* also rename the filter parameter to owner_id; repository and db_source callers are updated in the next slice so intermediate builds may fail in isolation.
7bac45f to
d151254
Compare
- api/gql_legacy/session.py: make ComputeSessionNode.from_dataclass synchronous again; main_access_key is now passed in by the caller (typically via UserRepository.get_main_access_key_by_id). Callers in ModifyComputeSession.mutate / CheckAndTransitStatus.mutate resolve it ahead of time and pass through. Avoids a hidden per-node DB round-trip and lets the caller batch the lookup. - models/keypair/row.py: drop the ``sessions`` relationship (and the join-condition helper + TYPE_CHECKING import) that was back-populating ``SessionRow.access_key_row`` — that relationship no longer exists on SessionRow after this slice. - models/resource_usage.py: rewrite stale TODO comments that said ``SessionRow.access_key has been removed`` — the column stays in this slice; it is deprecated and dropped in a later one. Also add ``UserRow.main_access_key`` to the ``load_only`` list so the field is hydrated when ``kern.session.user.main_access_key`` is read. - models/session/row.py: update ``match_sessions`` and ``get_session`` docstrings to describe ``owner_id`` instead of ``access_key``. Also drop the stale misc news fragment (slice is ``skip:changelog``).
After renaming SessionData/SessionMetadata fields and the SessionRow.get_session / match_sessions helpers, in-repo callers that used the old keyword argument (``access_key=``) or positional signature need the matching updates so slice C builds standalone: - manager/registry.py, repositories/stream/db_source/db_source.py, repositories/session/dependency_graph.py: switch to the new ``owner_id`` keyword argument on SessionRow.get_session / match_sessions. - models/endpoint/row.py: drop the ``target_access_key`` argument on SessionRow.delegate_ownership (its signature was trimmed by slice B). - tests/unit/manager/sokovan/scheduler/handlers/conftest.py: move SessionMetadata fixture fields from ``user_uuid``/``access_key`` to ``owner_id``; downstream access via ``metadata.owner_id`` and the rows now read ``main_access_key`` via the user record.
Slice C renames SessionData fields but left several downstream call sites broken in isolation (intentional per the original commit message). Fixing those so each PR in the stack passes its own CI: - ``SessionRow.match_sessions`` / ``get_session`` accept an optional ``owner_access_key`` keyword that resolves to the owner via the ``users.main_access_key`` sub-query — lets existing callers keep their AccessKey-based filter while the column rename completes. - ``find_dependency_sessions`` now takes ``owner: UUID | AccessKey`` and dispatches to the matching filter. - session/db_source call sites updated: positional AccessKey → keyword ``owner_access_key=…``. - Scheduler handlers test conftest temporarily uses ``access_key`` / ``user_uuid`` field names until the rename lands in slice F. - ``SessionAdapter._session_data_to_node`` reads ``data.owner_id`` and drops the obsolete ``data.access_key`` lookup. - ``ModelServingRepository.get_session_by_id`` drops the obsolete positional arg. - ``UserDBSource.delegate_endpoint_ownership`` discards the ``target_main_access_key`` arg (the ORM helper no longer uses it). - ``AgentRegistry`` stops threading ``user_repository`` into the ``SessionLifecycleManager`` constructor (the parameter is added in slice G). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Member
Author
|
Consolidated into the merged PR stack (A+B+C+D, E+F, G+H, I) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📚 Stack
mainMerge from top to bottom. Intermediate slices may not build standalone; the final refactor tip is #11051, and #11040 is the schema-drop follow-up on top of it.
Summary
Data-layer rename of
SessionData/SessionMetadata.user_uuid→owner_id; drop redundantaccess_keysnapshot fields.SessionRow.to_dataclass/from_dataclass/to_session_info/from_session_infoadapters updated.ComputeSessionNode.from_dataclassbecomes async, resolvesmain_access_keyvia UserRepository.models/resource_usage.pysourcesuser_idfromSessionRow.user_uuid._build_session_fetch_query/_match_sessions_by_*helpers rename filter parameter.Resolves BA-5711. Part of epic BA-5650.
📚 Documentation preview 📚: https://sorna--11045.org.readthedocs.build/en/11045/
📚 Documentation preview 📚: https://sorna-ko--11045.org.readthedocs.build/ko/11045/
BA-5650 Series: Split Rationale
Overall goal: migrate the session owner identifier from
access_key(keypair) toowner_id(user UUID), and drop thesessions.access_key/kernels.access_keycolumns.Split criteria: layer + dependency order. Bottom-up (DB helpers → service → API) so the destructive column drop can land safely at the end.
get_main_access_key_by_idand related resolver helpers (everything else depends on this)UserPermission.user_uuid → owner_id; addmain_access_keyfieldSessionData.user_uuid → owner_id; Row adapters; GQL nodeSessionRepository,SessionDBSource, creators signaturesowner_idfrom 21 read/control Actions; resolve viacurrent_user()owner_access_keyfrom REST v1 DTOs (breaking)Why this split
access_keyfromowner_id. BA-5653 (destructive) only runs once every reader has migrated off the dropped column.