Skip to content

refactor(BA-5709): add main_access_key resolver helpers#11041

Closed
jopemachine wants to merge 5 commits into
mainfrom
refactor/BA-5650-A-main-access-key-helpers
Closed

refactor(BA-5709): add main_access_key resolver helpers#11041
jopemachine wants to merge 5 commits into
mainfrom
refactor/BA-5650-A-main-access-key-helpers

Conversation

@jopemachine
Copy link
Copy Markdown
Member

@jopemachine jopemachine commented Apr 14, 2026

📚 Stack

Merge 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

Add UserRepository.get_main_access_key_by_id (plus the matching UserDBSource method) and rewrite SessionConditions.by_access_key_* filters to resolve the owner's main_access_key via a subquery. _owners_where_main_access_key staticmethod centralises the subquery shape. Behavior unchanged on main.

Resolves BA-5709. Part of epic BA-5650.


📚 Documentation preview 📚: https://sorna--11041.org.readthedocs.build/en/11041/


📚 Documentation preview 📚: https://sorna-ko--11041.org.readthedocs.build/ko/11041/


BA-5650 Series: Split Rationale

Overall goal: migrate the session owner identifier from access_key (keypair) to owner_id (user UUID), and drop the sessions.access_key / kernels.access_key columns.

Split criteria: layer + dependency order. Bottom-up (DB helpers → service → API) so the destructive column drop can land safely at the end.

Order Issue Layer Role
1 BA-5709 Foundation Add get_main_access_key_by_id and related resolver helpers (everything else depends on this)
2 BA-5710 RBAC / Permission Rename UserPermission.user_uuid → owner_id; add main_access_key field
3 BA-5711 Data Rename SessionData.user_uuid → owner_id; Row adapters; GQL node
4 BA-5712 Repository Collapse SessionRepository, SessionDBSource, creators signatures
5 BA-5713 Scheduler predicates / drf / options; scheduler repo; stream/events shims
6 BA-5714 Sokovan allocation / lifecycle / workload, launcher, controller, executor
7 BA-5715 Service Drop owner_id from 21 read/control Actions; resolve via current_user()
8 BA-5716 API (v1) Remove owner_access_key from REST v1 DTOs (breaking)
9 BA-5717 Tests / Cleanup Test suite updates; remaining ORM / gql_legacy touch-ups
10 BA-5653 Schema Alembic migration — rename/drop columns (destructive, must land last)

Why this split

  • Dependencies: the BA-5709 helpers are required before 3–7 can recover access_key from owner_id. BA-5653 (destructive) only runs once every reader has migrated off the dropped column.
  • Review size: a single-shot change would touch hundreds of files. Slicing by layer keeps each PR focused on one concern.
  • Rollback safety: steps 1–7 are no-op on external behavior, step 8 is the API breaking change, step 10 is the schema drop. Each step is independently revertible.

Copilot AI review requested due to automatic review settings April 14, 2026 07:07
@github-actions github-actions Bot added size:M 30~100 LoC comp:manager Related to Manager component labels Apr 14, 2026
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

Note

Copilot was unable to run its full agentic suite in this review.

This PR adds helper APIs to resolve a user’s main_access_key and rewires session access-key filters to resolve via the owning user’s main_access_key (preparatory work for dropping redundant sessions.access_key / kernels.access_key columns).

Changes:

  • Add UserRepository.get_main_access_key_by_id and UserDBSource.get_main_access_key_by_id for single-query access-key resolution by user UUID.
  • Rewrite SessionConditions.by_access_key_* filters to use a UserRow.main_access_key subquery keyed by SessionRow.user_uuid.
  • Add a changelog entry describing the refactor groundwork.

Reviewed changes

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

File Description
src/ai/backend/manager/repositories/user/repository.py Adds repository helper for main_access_key lookup and adjusts endpoint ownership delegation call signature.
src/ai/backend/manager/repositories/user/db_source/db_source.py Adds DBSource scalar query for main_access_key and updates endpoint ownership delegation call signature.
src/ai/backend/manager/models/session/conditions.py Reworks access-key session filters to resolve via owner subquery; adds centralized helper.
changes/11041.misc.md Documents the new helper + filter rewrite as groundwork for future column removal.

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

Comment thread src/ai/backend/manager/models/session/conditions.py Outdated
Comment thread src/ai/backend/manager/models/session/conditions.py
Comment thread src/ai/backend/manager/repositories/user/repository.py
Comment thread src/ai/backend/manager/repositories/user/db_source/db_source.py Outdated
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-A-main-access-key-helpers branch from 2d7d64e to e66536b Compare April 14, 2026 07:38
Comment thread changes/11041.misc.md
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).
@jopemachine jopemachine added the skip:changelog Make the action workflow to skip towncrier check label Apr 14, 2026
@jopemachine jopemachine removed the skip:changelog Make the action workflow to skip towncrier check label Apr 14, 2026
@jopemachine
Copy link
Copy Markdown
Member Author

Consolidated into the merged PR stack (A+B+C+D, E+F, G+H, I)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:docs Documentations comp:manager Related to Manager component size:M 30~100 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants