Skip to content

refactor(BA-5650): add owner_id helpers and rename session data types [1/4]#11046

Closed
jopemachine wants to merge 22 commits into
mainfrom
refactor/BA-5650-D-session-repo-owner-id
Closed

refactor(BA-5650): add owner_id helpers and rename session data types [1/4]#11046
jopemachine wants to merge 22 commits into
mainfrom
refactor/BA-5650-D-session-repo-owner-id

Conversation

@jopemachine
Copy link
Copy Markdown
Member

@jopemachine jopemachine commented Apr 14, 2026

Resolves #10914 (BA-5650)

Summary

  • Add main_access_key resolver helpers for user/keypair lookup
  • Thread main_access_key through UserPermission replacing raw AccessKey
  • Rename SessionData.user_uuidowner_id, remove access_key field
  • Update session repository to use owner_id consistently

Test plan

  • pants check passes on this slice
  • Unit tests for session repository pass

Stack

  1. refactor(BA-5650): add owner_id helpers and rename session data types [1/4] #11046 ← You are here — add owner_id helpers and rename session data types
  2. refactor(BA-5650): propagate owner_id through scheduler and sokovan [2/4] #11048 — propagate owner_id through scheduler and sokovan
  3. refactor(BA-5650): unify session ownership to owner_id and main_access_key [1/2] #11050 — resolve owner_id in services and drop REST v1 owner_access_key
  4. refactor(BA-5650): update tests and remaining ORM references [2/2] #11051 — update tests and remaining ORM references

🤖 Generated with Claude Code

Two independent additions that other parts of the BA-5650 stack build
on:

- ``UserRepository.get_main_access_key_by_id`` (+ the matching
  ``UserDBSource`` method) returns ``main_access_key`` for a given
  user UUID with a single readonly query. Used by downstream refactors
  that need to resolve the keypair access_key on demand from the owner.
- ``SessionConditions.by_access_key_*`` filters in
  ``models/session/conditions.py`` now resolve through a subquery on
  ``UserRow.main_access_key`` keyed by ``SessionRow.user_uuid``, instead
  of the ``sessions.access_key`` column. A new
  ``_owners_where_main_access_key`` staticmethod centralises the
  subquery shape.

Behavior is unchanged on ``main`` where ``sessions.access_key`` still
mirrors the owner's main_access_key; this change just lets us drop the
redundant column later.
Slice A scope is only the new get_main_access_key_by_id helper and the
SessionConditions rewrite. Accidentally dropping the
target_main_access_key argument from delegate_endpoint_ownership
belongs to a later slice that also updates EndpointRow and the user
service; restoring the signature here so this PR builds standalone.
@jopemachine jopemachine force-pushed the refactor/BA-5650-C-session-data-owner-id branch from 0b27565 to caa8001 Compare April 14, 2026 07:39
@jopemachine jopemachine force-pushed the refactor/BA-5650-D-session-repo-owner-id branch from 719ff4b to f811d03 Compare April 14, 2026 07:39
@jopemachine jopemachine requested a review from Copilot April 14, 2026 07:46
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

Updates the session repository/data-source layer to use an owner UUID (owner_id) instead of an access key for session lookups and dependency resolution, aligning with the ongoing BA-5650 ownership refactor.

Changes:

  • Refactor SessionRepository and SessionDBSource method signatures to accept owner_id: UUID (e.g., session lookup/matching, rename, dependency traversal).
  • Update dependency-graph helpers to traverse and validate sessions using owner_id.
  • Adjust related documentation/changelog entry for the signature shift.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/ai/backend/manager/repositories/session/repository.py Switch repository APIs from owner_access_key to owner_id and forward to DB source.
src/ai/backend/manager/repositories/session/db_source/db_source.py Switch DB source session lookup/update/dependency APIs to owner_id; update internal call sites.
src/ai/backend/manager/repositories/session/dependency_graph.py Replace access-key scoping with owner UUID scoping for dependency traversal.
src/ai/backend/manager/repositories/session/creators.py Update documentation wording related to ownership scope (needs correction per comment).
changes/11046.misc.md Add changelog note describing the signature collapse to owner_id.

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

Comment thread src/ai/backend/manager/repositories/session/creators.py Outdated
Comment thread src/ai/backend/manager/repositories/session/db_source/db_source.py Outdated
Comment thread src/ai/backend/manager/repositories/session/db_source/db_source.py
Comment thread changes/11046.misc.md Outdated
@jopemachine jopemachine force-pushed the refactor/BA-5650-C-session-data-owner-id branch from caa8001 to 7bac45f Compare April 14, 2026 07:52
@jopemachine jopemachine force-pushed the refactor/BA-5650-D-session-repo-owner-id branch from f811d03 to c2d0d94 Compare April 14, 2026 07:52
Drop the redundant UserRow.main_access_key IS NOT NULL guard inside
SessionConditions._owners_where_main_access_key. The subquery selects
users.uuid (non-null PK), so NOT IN is well-defined; NULL
main_access_key fails the caller-supplied condition on its own. The
behaviour is unchanged — this just makes the helper contract clearer.

Also drop the stale misc news fragment (skip:changelog applies to this
purely additive refactor).
Rename UserPermission.user_uuid to owner_id and add main_access_key:
str | None. KernelRow.to_kernel_info populates the new field from the
eagerly-loaded user_row relationship; search_kernels now selectinload's
user_row so the scalar is available without N+1.

KernelRow.delegate_ownership drops its access_key argument (resolved
from the owner). recalc_concurrency_used switches from
KernelRow.access_key to a KernelRow.user_uuid subquery keyed by the
owner's main_access_key as a shim for the eventual access_key column
drop.

Test fixtures that construct UserPermission are updated.
Earlier the branch pulled test fixtures that also referenced later
slices' renames (SessionMetadata.owner_id, ScheduledSessionData
.main_access_key, SessionDataForPull/Start.main_access_key,
SessionData.owner_id). Revert those fixtures to main state and
re-apply only the UserPermission field renames (user_uuid -> owner_id,
access_key -> main_access_key) which are in this slice's scope.

Also update the two remaining live readers of UserPermission:
- sokovan/scheduler/fair_share/aggregator.py
- api/adapters/session.py (kernel_info_to_node)

And fix the SessionRow.delegate_ownership caller of
KernelRow.delegate_ownership to match the new single-argument signature.
@jopemachine jopemachine force-pushed the refactor/BA-5650-D-session-repo-owner-id branch from 2caf306 to c1a5f32 Compare April 14, 2026 08:07
@github-actions github-actions Bot added size:L 100~500 LoC and removed size:M 30~100 LoC labels Apr 14, 2026
Comment thread changes/BA-5650-C.misc.md Outdated
Comment thread src/ai/backend/manager/repositories/session/db_source/db_source.py Outdated
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.
@jopemachine jopemachine force-pushed the refactor/BA-5650-D-session-repo-owner-id branch from e2ff93b to 765d164 Compare April 14, 2026 08:27
@github-actions github-actions Bot added size:M 30~100 LoC and removed size:L 100~500 LoC labels Apr 14, 2026
jopemachine and others added 8 commits April 14, 2026 19:14
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>
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.
``SessionRepository`` and the underlying ``SessionDBSource`` now take
``owner_id: UUID`` on every method that previously accepted
``owner_access_key: AccessKey``. Affects:

- ``get_session_validated``
- ``match_sessions``
- ``update_session_name``
- ``find_dependency_sessions`` / ``_find_dependent_sessions``
- ``get_target_session_ids``
- ``get_session_with_group``

The matching ``dependency_graph`` helpers and ``creators`` are updated
in lockstep. Service-layer callers still pass ``owner_access_key``
temporarily; they will be migrated in the next slice.
- repositories/session/creators.py: docstring pointed to a non-existent
  ``owner_id`` attribute on SessionRow; clarify that the scope_id
  comes from ``user_uuid`` (the owner's UUID).
- repositories/session/db_source/db_source.py: drop the redundant
  ``session_row.user_uuid is not None`` guard (the column is
  non-nullable); update the ``_find_dependent_sessions`` docstring to
  reference ``owner_id`` instead of the removed ``access_key`` param.
- Drop the leftover ``changes/BA-5650-C.misc.md`` file that was
  superseded when slice C's fragment was renamed to
  ``changes/11045.enhance.md``.
- Remove the explanatory comment about ``user_uuid`` being non-nullable
  from the ``update_session_name`` path — it restates what the
  surrounding code already makes clear.
After rebasing on slice C, three residual references in
``session/db_source/db_source.py`` still used the old ``access_key``
variable name. This commit finishes the rename in:

- ``get_session_validated`` / ``match_sessions`` / ``update_session_name``
  (now forward ``owner_id=...`` instead of the leftover
  ``owner_access_key`` keyword).
- ``_find_dependent_sessions`` and ``get_target_session_ids`` (now pass
  the local ``owner_id`` UUID instead of the non-existent ``access_key``).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jopemachine jopemachine force-pushed the refactor/BA-5650-D-session-repo-owner-id branch from 4ecaf01 to 82ea2d0 Compare April 14, 2026 10:16
@jopemachine jopemachine changed the base branch from refactor/BA-5650-C-session-data-owner-id to main April 15, 2026 01:41
@jopemachine jopemachine changed the title refactor(BA-5712): switch session repository to owner_id refactor(BA-5650): add owner_id helpers and rename session data types [1/4] Apr 15, 2026
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size:L 100~500 LoC and removed size:M 30~100 LoC labels Apr 15, 2026
@jopemachine jopemachine added this to the 26.5 milestone Apr 15, 2026
@jopemachine
Copy link
Copy Markdown
Member Author

Squashed into PR #11051 — stacked PRs cannot pass CI independently with cross-cutting type changes.

@jopemachine jopemachine reopened this Apr 15, 2026
@jopemachine
Copy link
Copy Markdown
Member Author

Restructured into 2 self-contained PRs (#11050, #11051) for CI pass.

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.

Rename sessions/kernels.user_uuid to owner_id and drop access_key columns

2 participants