refactor(BA-5979): split deployment search into admin and scoped layers#11522
refactor(BA-5979): split deployment search into admin and scoped layers#11522jopemachine wants to merge 6 commits into
Conversation
6f84f80 to
4b8f701
Compare
There was a problem hiding this comment.
Pull request overview
Splits the deployment search/projection paths into a dedicated admin layer plus scoped (user / project) actions. The v2 API path now projects EndpointRow directly to ModelDeploymentData via a new to_model_deployment_data() method, bypassing the DeploymentInfo intermediate. A new DeploymentAdminRepository + DeploymentAdminService + DeploymentAdminProcessors package mirrors the vfolder/login-admin convention, and my_search / project_search get their own scopes and actions. The legacy v1 POST /deployments/search keeps its URL but now requires project_id in the body.
Changes:
- New
EndpointRow.to_model_deployment_data()andModelDeploymentStatus.from_lifecycle(); service-side_convert_deployment_info_to_dataremoved. - New
DeploymentAdminRepository/DeploymentAdminService/DeploymentAdminProcessors+AdminSearchDeploymentsAction; user/project scoped search actions andUserDeploymentSearchScopeadded; legacy v1 search routed through project scope. - DTO renames:
AdminSearchDeploymentsInput/Payload→SearchDeploymentsInput/Payload;SearchDeploymentsRequest→SearchLegacyDeploymentsRequest(requiresproject_id); CLIbai deployment list --project-idnow required.
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ai/backend/manager/models/endpoint/row.py | Adds to_model_deployment_data() projection. |
| src/ai/backend/common/data/model_deployment/types.py | Adds ModelDeploymentStatus.from_lifecycle(). |
| src/ai/backend/manager/data/deployment/types.py | Renames DeploymentInfoSearchResult → ModelDeploymentDataSearchResult. |
| src/ai/backend/manager/repositories/deployment/db_source/db_source.py | Drops storage_manager from DBSource; adds get_deployment_data, admin_search_deployments, search_user_deployments, renames project search methods. |
| src/ai/backend/manager/repositories/deployment/admin_repository.py | New admin repository owning unscoped search. |
| src/ai/backend/manager/repositories/deployment/repository.py / repositories.py / init.py | Wire admin repository, add new scoped search methods. |
| src/ai/backend/manager/repositories/deployment/types/endpoint.py / init.py | New UserDeploymentSearchScope. |
| src/ai/backend/manager/services/deployment/admin_service.py | New admin service. |
| src/ai/backend/manager/services/deployment/processors.py | Adds DeploymentAdminProcessors and scoped processors. |
| src/ai/backend/manager/services/deployment/service.py | Removes legacy converter; switches create/update/get/activate to get_deployment_data; adds scoped search service methods. |
| src/ai/backend/manager/services/deployment/actions/admin_search_deployments.py / search_user_deployments.py / search_project_deployments.py / search_project_deployment_summary.py | Action classes for the new split. |
| src/ai/backend/manager/services/factory.py / processors.py | Register admin service + processors. |
| src/ai/backend/manager/api/adapters/deployment/adapter.py | Routes admin_search / my_search / project_search / batch_load_by_ids to the new actions with scopes. |
| src/ai/backend/manager/api/rest/v2/deployment/handler.py | Uses renamed SearchDeploymentsInput. |
| src/ai/backend/manager/api/rest/deployment/{handler,adapter}.py | Legacy v1 search uses SearchLegacyDeploymentsRequest and routes through project-scoped action. |
| src/ai/backend/manager/api/gql/deployment/resolver/deployment.py | Renamed input class. |
| src/ai/backend/common/dto/manager/{deployment,v2/deployment}/* | DTO renames and shared SearchDeploymentsInput/Payload. |
| src/ai/backend/common/metrics/metric.py | Adds DEPLOYMENT_ADMIN_REPOSITORY layer. |
| src/ai/backend/client/** | Client/CLI renames; bai deployment list now requires --project-id. |
| tests/unit/manager/repositories/deployment/test_endpoint_projection.py | New unit tests for the projection. |
| tests/unit/manager/repositories/deployment/test_search_project_deployment_summary.py | Renames to match new repository method. |
| tests/unit/manager/services/deployment/test_deployment_service.py | Removes tests for deleted converter. |
| tests/component/deployment/conftest.py | Adds RBAC fixture granting PROJECT-scoped MODEL_DEPLOYMENT:READ. |
| tests/component/deployment/test_deployment*.py, tests/unit/client_v2/test_deployment.py | Adopt renamed inputs and new RBAC fixture. |
| changes/11522.enhance.md | News fragment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async with self._begin_readonly_session_read_committed() as db_sess: | ||
| query = ( | ||
| sa.select(EndpointRow) | ||
| .where(EndpointRow.id == endpoint_id) | ||
| .options( | ||
| selectinload(EndpointRow.revisions).selectinload( | ||
| DeploymentRevisionRow.image_row | ||
| ), | ||
| selectinload(EndpointRow.deployment_policy), | ||
| ) | ||
| ) | ||
| result = await db_sess.execute(query) | ||
| row: EndpointRow | None = result.scalar_one_or_none() | ||
|
|
||
| if not row: | ||
| raise EndpointNotFound(f"Endpoint {endpoint_id} not found") | ||
|
|
||
| return row.to_model_deployment_data() |
| case EndpointLifecycle.DESTROYING: | ||
| return cls.STOPPING | ||
| case EndpointLifecycle.DESTROYED: | ||
| return cls.STOPPED |
| ) -> list[DeploymentNode | None]: | ||
| """Batch load deployments by ID for DataLoader use. | ||
|
|
||
| Returns DeploymentNode DTOs in the same order as the input deployment_ids list. | ||
| Routed through ``admin_search_deployments`` — the only remaining | ||
| unscoped search after the legacy path was removed. The ``by_ids`` | ||
| filter is the bound on the result set; the parent resolver has | ||
| already authorised access to whatever entity references these IDs, | ||
| and the action is unscoped at the service layer (the admin label | ||
| is enforced at the resolver, not here). | ||
|
|
||
| Output is aligned with the input ``deployment_ids`` order; missing | ||
| IDs come back as ``None``. | ||
| """ | ||
| if not deployment_ids: | ||
| return [] | ||
| querier = BatchQuerier( | ||
| pagination=OffsetPagination(limit=len(deployment_ids)), | ||
| conditions=[DeploymentConditions.by_ids(deployment_ids)], | ||
| ) | ||
| action_result = await self._processors.deployment.search_deployments.wait_for_complete( | ||
| SearchDeploymentsAction(querier=querier) | ||
| action_result = ( | ||
| await self._processors.deployment_admin.admin_search_deployments.wait_for_complete( | ||
| AdminSearchDeploymentsAction(querier=querier) | ||
| ) | ||
| ) |
| def _build( | ||
| *, | ||
| current_revision: DeploymentRevisionID | None = None, | ||
| deploying_revision: DeploymentRevisionID | None = None, | ||
| current_revision_row: Any = None, | ||
| deploying_revision_row: Any = None, | ||
| lifecycle_stage: EndpointLifecycle = EndpointLifecycle.DEPLOYING, | ||
| ) -> Any: | ||
| stub = MagicMock() | ||
| stub.id = DeploymentID(uuid.uuid4()) | ||
| stub.name = "test-deployment" | ||
| stub.lifecycle_stage = lifecycle_stage | ||
| stub.tag = None | ||
| stub.project = uuid.uuid4() | ||
| stub.domain = "default" | ||
| stub.created_at = datetime(2024, 1, 1, tzinfo=UTC) | ||
| stub.open_to_public = False | ||
| stub.url = None | ||
| stub.current_revision = current_revision | ||
| stub.deploying_revision = deploying_revision | ||
| stub.current_revision_row = current_revision_row | ||
| stub.deploying_revision_row = deploying_revision_row | ||
| stub.replicas = 1 | ||
| stub.desired_replicas = None | ||
| stub.deployment_policy = None | ||
| stub.created_user = uuid.uuid4() | ||
| stub.options = DeploymentOptions() | ||
| stub.scaling_state = ScalingState.STABLE | ||
| stub.sub_step = None | ||
| return stub | ||
|
|
||
| return _build |
db1f00b to
4b8f701
Compare
Restructures the deployment search/projection stack so every layer (db_source → repository → service → processor → adapter → REST/GQL/SDK/CLI) makes the search scope explicit: - Admin-only search: ``DeploymentAdminRepository``/``DeploymentAdminService`` with ``admin_search_deployments`` at every level. The DBSource side carries the prefix too so the unscoped intent is visible alongside the scoped ``search_user_deployments`` / ``search_project_deployments`` variants in the same namespace. - User/project search: dedicated actions (``SearchUserDeploymentsAction``, ``SearchProjectDeploymentsAction``) with their own RBAC scope. The v2 adapter's ``my_search`` / ``project_search`` now route through these. - Project summary: ``SearchProjectDeploymentSummaryAction`` for the lightweight project-admin list view (renamed from ``SearchDeploymentsInProject``). EndpointRow → ModelDeploymentData projection moves onto the row as ``EndpointRow.to_model_deployment_data()``, following the existing ``to_deployment_info`` shape and the BA-6056 relationship split: the projection reads ``current_revision_row`` / ``deploying_revision_row`` directly instead of scanning a ``revisions`` list. The fragile ``DeploymentInfo``-via-helper path (``_convert_deployment_info_to_data``) disappears together with its placeholder defaults; service callers now fetch ``ModelDeploymentData`` straight from the repository. API surface: - v1 REST: ``POST /deployments/search`` contract preserved; a new ``SearchLegacyDeploymentsRequest`` (standalone, not a subclass) carries the ``project_id`` inline on the body, and the handler routes through ``SearchProjectDeploymentsAction``. v1 SDK, ``./bai`` CLI, and ``client/func`` migrate to the new request type. - v2 REST/GQL: ``SearchDeploymentsInput`` / ``SearchDeploymentsPayload`` (neutral names, no admin prefix) are shared by admin / project / my variants — mirrors the vfolder / model_card / user / session v2 convention. Admin and scope variants only differ in URL + middleware. - GraphQL ``deployment_loader`` keeps the unscoped batch lookup it was born with, now routed through ``admin_search_deployments`` rather than the legacy slot — parent resolvers stay responsible for RBAC. Tests: - ``tests/unit/manager/repositories/deployment/test_endpoint_projection.py`` pins the new ``EndpointRow.to_model_deployment_data`` against the ``current_revision_row`` relationship (replaces the BA-5963 list-order regression test, which is structurally impossible under the new lookup). - v1 component / unit / SDK tests migrate to ``SearchLegacyDeploymentsRequest``. - Existing ``SearchProjectDeploymentSummary`` action becomes ``@dataclass(frozen=True)`` to match the rest of the search-action set. Co-Authored-By: Gyubong <gbl@lablup.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The DBSource's ``get_deployment_data`` was still selectinload-ing ``EndpointRow.revisions`` while the projection method ``to_model_deployment_data`` reads ``current_revision_row`` / ``deploying_revision_row`` directly. Those relationships default to ``lazy="select"``, so accessing them on a row fetched under an async session would trip the SQLAlchemy ``greenlet_spawn`` error on every API path that calls ``get_deployment_data`` (``create_deployment``, ``update_deployment``, ``get_deployment_by_id``, ``activate_revision``). Align the eager-load with what ``admin_search_deployments`` / ``search_user_deployments`` / ``search_project_deployments`` already do, and drop the now-unused ``revisions`` chain. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… shape
Drop nested revision / policy / id-list payloads from
``ModelDeploymentData`` so the projection only ever carries the endpoint
row's own columns. The v2 GQL node already exposes only scope IDs and
defers the revision spec / policy / replica list / auto-scaling rules /
access tokens to dedicated DataLoader resolvers; the v1 REST surface
now mirrors that — clients fetch the spec through the nested endpoints
(``/deployments/{id}/revisions/{revision_id}``,
``/deployments/{id}/policy``, etc.). **Breaking change for v1 REST**:
``DeploymentDTO.current_revision`` and ``DeploymentDTO.deployment_policy``
are removed; ``current_revision_id`` and ``deploying_revision_id`` are
exposed instead.
With the projection no longer touching ``EndpointRow.current_revision_row``
/ ``deploying_revision_row`` / ``deployment_policy``, the four search /
get paths that consume ``to_model_deployment_data`` drop their
``selectinload`` chains — each was incurring a per-row dead eager load.
The v2 ``DeploymentNode.policy`` field is removed for the same reason
(GQL never read it; the resolver always went through the policy
DataLoader). The v1 handler's revision-variant resolver is no longer
needed on the deployment DTO path; ``_deployment_dto`` becomes
synchronous.
Tests:
- ``test_endpoint_projection`` drops the now-obsolete revision-row
scenarios (the BA-5963 list-order regression is structurally
impossible once the projection only reads columns) and keeps the
column-pass-through plus the lifecycle status mapping pins.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…factor a70582c dropped the ``policy`` field from ``DeploymentNode`` (v2 GQL response) but left this test module still building / asserting on it, so mypy hit ``"DeploymentNode" has no attribute "policy"`` in 7 spots and the typecheck CI job failed. Remove the ``policy`` default from the test factory, drop the three now-meaningless policy test cases, and prune the imports they exclusively used. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The v1 ``DeploymentDTO.sub_step`` had no v2 counterpart, so the GQL ``ModelDeployment`` node and the v2 REST ``DeploymentNode`` were missing the rolling-update progress signal. Promote ``DeploymentLifecycleSubStep`` to ``common.data.model_deployment.types`` so the v2 DTO layer can reach it (``common/`` cannot import from ``manager/``), repoint every existing importer at the common location, then surface the enum on: - ``DeploymentNode.sub_step`` (v2 REST DTO) - ``ModelDeployment.subStep`` (GQL, backed by a new ``DeploymentLifecycleSubStep`` GQL enum) The v2 adapter pipes ``ModelDeploymentData.sub_step`` through to the new DTO field, and the v2 schema dump picks up the enum + field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
0c2a928 to
646ffdb
Compare
CI typecheck on the previous commit failed because three sokovan deployment test files still pulled ``DeploymentLifecycleSubStep`` from ``ai.backend.manager.data.deployment.types`` — that re-export was the casualty of moving the enum to ``common.data.model_deployment.types``. Update the test imports to the common location to match every other manager/sokovan caller migrated in the previous commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Split the deployment search/projection paths so each axis (admin / user / project / project-summary / GraphQL DataLoader) has a dedicated action and repository method.
create/update/get/activate_revisionreads throughEndpointRow.to_model_deployment_data()directly at thedb_source/boundary, bypassing theDeploymentInfointermediate.DeploymentAdminRepository+DeploymentAdminService+DeploymentAdminProcessorspackage, mirroring thevfolder/login_client_typeadmin-split convention.my_search) and project-scoped (project_search) reads each get their ownSearch{User,Project}DeploymentsActionand a{User,Project}DeploymentSearchScope; the scope filter lives in the repository, not on the adapter as an injectedbase_condition.POST /deployments/searchkeeps its no-path-segment shape so existing CLI/SDK callers don't break — the project scope now travels inline on the body via the newSearchLegacyDeploymentsRequest(justSearchDeploymentsRequest+ a requiredproject_id), and the handler resolves it to aProjectDeploymentSearchScopeand routes through the samesearch_project_deploymentsprocessor as v2.batch_load_by_ids) routes throughadmin_search_deployments(the only remaining unscoped search after the legacy no-scope action was dropped). The parent resolver has already authorised access to whatever references these IDs, so the admin processor is acceptable here — the admin label is enforced at the resolver, not at the DataLoader.Resolves BA-5979. Builds on top of #11494 (BA-5963) which is already on
main.Layer-by-layer changes
Models
models/endpoint/row.pyto_deployment_infoonlyto_model_deployment_data()— projects directly toModelDeploymentData(used by every new ModelDeploymentData-returning DB-source method)Repository
DeploymentRepositoryget_endpoint_infoDeploymentInfoDeploymentRepositoryget_deployment_dataModelDeploymentDatafor the API pathDeploymentRepositorysearch_endpointsDeploymentInfoDeploymentRepositorysearch_user_deploymentsModelDeploymentDataDeploymentRepositorysearch_project_deploymentsModelDeploymentDataDeploymentRepositorysearch_project_deployment_summarysearch_deployments_in_project) — still backs project admin list pages withDeploymentSummaryDataDeploymentAdminRepositorysearch_deploymentsModelDeploymentDatarepositories/deployment/db_source/db_source.pyEndpointRow.to_model_deployment_data()DeploymentInfoDeploymentDBSource._storage_managerDeploymentStorageSource(still owned byDeploymentRepository)Scopes
ProjectDeploymentSearchScopeUserDeploymentSearchScopeEndpointRow.created_user == user_idAction
SearchDeploymentsActionAdminSearchDeploymentsActionSearchUserDeploymentsActionSearchProjectDeploymentsActionModelDeploymentDataSearchProjectDeploymentSummaryActionSearchDeploymentsInProjectActionDeploymentSummaryDatafor project admin list pagesService
DeploymentServicecreate_deploymentget_endpoint_info+_convert_deployment_info_to_dataget_deployment_dataDeploymentServiceupdate_deploymentDeploymentInfo→ convertget_deployment_dataDeploymentServiceget_deployment_by_idget_endpoint_info+ convertget_deployment_dataDeploymentServiceactivate_revisionDeploymentInfo→ convertget_deployment_dataDeploymentServicesearch_user_deploymentsDeploymentServicesearch_project_deploymentsModelDeploymentData; serves both the v2 adapter and the legacy v1 REST handlerDeploymentServicesearch_project_deployment_summarysearch_deployments_in_project; still returns the lighterDeploymentSummaryDataDeploymentAdminServiceadmin_search_deploymentsDeploymentAdminRepository.search_deploymentsDeploymentService(private)_convert_deployment_info_to_dataProcessor
DeploymentProcessorssearch_deploymentsDeploymentProcessorssearch_user_deploymentsDeploymentProcessorssearch_project_deploymentsDeploymentProcessorssearch_project_deployment_summarysearch_deployments_in_projectDeploymentAdminProcessors(new)admin_search_deploymentsProcessorsAdapter routing
admin_searchSearchDeploymentsAction(regular processor)AdminSearchDeploymentsAction(admin processor)my_searchSearchDeploymentsAction+created_user==user_idbase-conditionSearchUserDeploymentsAction+UserDeploymentSearchScope(regular processor)project_searchSearchDeploymentsAction+project==project_idbase-conditionSearchProjectDeploymentsAction+ProjectDeploymentSearchScope(regular processor)batch_load_by_ids(DataLoader)SearchDeploymentsAction+by_idsconditionAdminSearchDeploymentsAction+by_idscondition (admin processor; the parent resolver already authorised access)Legacy v1 REST handler
api/rest/deployment/handler.pyPOST /deployments/searchkeeps its no-path-segment URL but now takes aSearchLegacyDeploymentsRequestbody (project_id required), builds aProjectDeploymentSearchScope, and routes through the sharedsearch_project_deploymentsprocessor. RBAC therefore now enforces project-scopedMODEL_DEPLOYMENT:READon this endpoint.api/rest/tree.pyorigin/mainclient/cli/deployment.py./bai deployment listnow requires--project-id.Tests
tests/unit/manager/repositories/deployment/test_endpoint_projection.pyEndpointRow.to_model_deployment_data()covering reversedrevisionsorder, danglingcurrent_revisionreferences, and the lifecycle status mappingtests/component/deployment/conftest.pyregular_user_project_model_deployment_read_permissionfixture — grants PROJECT-scopedMODEL_DEPLOYMENT:READtoregular_user_fixtureso legacy-path regular-user search tests can pass the now-enforced RBAC checkTest plan
pants fmt fix lint checkon every changed filetests/unit/manager/repositories/deployment/test_endpoint_projection.pytests/component/deployment/test_deployment_lifecycle.py::TestUserAccessDeployment::test_user_searches_empty_deployments(now seeds the project-scoped read role; previously passed only because the action was unscoped)./baismoke after merge: admin search, my search, project search, GraphQLmodelDeploymentresolver (DataLoader), legacyPOST /deployments/search🤖 Generated with Claude Code
📚 Documentation preview 📚: https://sorna--11522.org.readthedocs.build/en/11522/
📚 Documentation preview 📚: https://sorna-ko--11522.org.readthedocs.build/ko/11522/