enhance(BA-4596): Add DTO which can be shared among different end point#9244
enhance(BA-4596): Add DTO which can be shared among different end point#9244
Conversation
github-actions
bot
added
size:L
There was a problem hiding this comment.
Pull request overview
This pull request adds shared DTO (Data Transfer Object) definitions for resource slot endpoints that can be used across different API implementations (GraphQL, REST, SDK). The PR introduces DTOs in the common package for agent resources and kernel resource allocations, along with repository-layer query conditions and ordering options.
Changes:
- Added DTO package structure for resource slots with request, response, and type definitions
- Implemented repository query conditions and orders for AgentResource and ResourceAllocation tables
- Created filter and ordering types to support flexible search and pagination
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ai/backend/common/dto/manager/resource_slot/init.py | Package exports for all DTO classes, request/response models, and types |
| src/ai/backend/common/dto/manager/resource_slot/types.py | Defines filter types (AgentResourceFilter, ResourceAllocationFilter), order types, and field enums |
| src/ai/backend/common/dto/manager/resource_slot/request.py | Request DTOs for search and get operations with path parameters and pagination |
| src/ai/backend/common/dto/manager/resource_slot/response.py | Response DTOs with agent resource and allocation data structures plus pagination info |
| src/ai/backend/manager/repositories/resource_slot/options.py | Repository query conditions and orders for database operations on agent resources and allocations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class SearchAgentResourcesRequest(BaseRequestModel): | ||
| filter: AgentResourceFilter | None = Field(default=None, description="Filter conditions") | ||
| order: list[AgentResourceOrder] | None = Field(default=None, description="Order specification") | ||
| limit: int = Field(default=20, ge=1, le=100, description="Maximum items to return") |
There was a problem hiding this comment.
The pagination limit values should use shared constants from ai.backend.common.dto.manager.defs rather than hardcoded values. The default should be DEFAULT_PAGE_LIMIT (50) and the maximum should be MAX_PAGE_LIMIT (1000) to maintain consistency with other search endpoints across the codebase. This ensures uniform pagination behavior and makes it easier to adjust these values globally if needed in the future.
| order: list[ResourceAllocationOrder] | None = Field( | ||
| default=None, description="Order specification" | ||
| ) | ||
| limit: int = Field(default=20, ge=1, le=100, description="Maximum items to return") |
There was a problem hiding this comment.
The pagination limit values should use shared constants from ai.backend.common.dto.manager.defs rather than hardcoded values. The default should be DEFAULT_PAGE_LIMIT (50) and the maximum should be MAX_PAGE_LIMIT (1000) to maintain consistency with other search endpoints across the codebase. This ensures uniform pagination behavior and makes it easier to adjust these values globally if needed in the future.
kwonkwonn
force-pushed
the
feat/resource-slot-DTO
branch
from
cf2a039 to
f0c896a
Compare
| from ai.backend.manager.models.resource_slot import AgentResourceRow, ResourceAllocationRow | ||
| from ai.backend.manager.repositories.base.types import QueryCondition, QueryOrder | ||
|
|
||
|
|
There was a problem hiding this comment.
Im not particulary sure for this options,
If there's misleaded things let me know!
| class AgentResourceConditions: | ||
| @staticmethod | ||
| def by_agent_id(agent_id: str) -> QueryCondition: | ||
| def inner() -> sa.sql.expression.ColumnElement[bool]: | ||
| return AgentResourceRow.agent_id == agent_id | ||
|
|
||
| return inner | ||
|
|
||
| @staticmethod | ||
| def by_slot_name(slot_name: str) -> QueryCondition: | ||
| def inner() -> sa.sql.expression.ColumnElement[bool]: | ||
| return AgentResourceRow.slot_name == slot_name | ||
|
|
||
| return inner |
There was a problem hiding this comment.
This doesn't seem to be working. Could you take a look at the StringFilter part? I've been attaching it recently to match the StringMatcherSpec.
|
All migration tasks have been applied; I will now close this. |
resolves #9242 (BA-4596)
DTO can be shared among different end-points.
we need to have one before implementing gql, rest, sdk
Checklist: (if applicable)
ai.backend.testdocsdirectory