diff --git a/changes/9673.fix.md b/changes/9673.fix.md new file mode 100644 index 00000000000..9df5518cdf5 --- /dev/null +++ b/changes/9673.fix.md @@ -0,0 +1 @@ +Incorrect types and fallback defaults in deployment conversion logic diff --git a/src/ai/backend/common/dto/manager/deployment/response.py b/src/ai/backend/common/dto/manager/deployment/response.py index b25e8082fa3..4302718182e 100644 --- a/src/ai/backend/common/dto/manager/deployment/response.py +++ b/src/ai/backend/common/dto/manager/deployment/response.py @@ -94,7 +94,7 @@ class ModelMountConfigDTO(BaseModel): """Model mount configuration for revision.""" vfolder_id: UUID = Field(description="VFolder ID for model") - mount_destination: str = Field(description="Mount destination path") + mount_destination: str | None = Field(description="Mount destination path") definition_path: str = Field(description="Model definition path") diff --git a/src/ai/backend/manager/api/gql/deployment/types/revision.py b/src/ai/backend/manager/api/gql/deployment/types/revision.py index 00f566dd9d7..2cea0dd01c3 100644 --- a/src/ai/backend/manager/api/gql/deployment/types/revision.py +++ b/src/ai/backend/manager/api/gql/deployment/types/revision.py @@ -515,11 +515,9 @@ def to_model_revision_creator(self) -> ModelRevisionCreator: extra_mounts = [ MountInfo( vfolder_id=UUID(str(extra_mount.vfolder_id)), - kernel_path=PurePosixPath( - extra_mount.mount_destination - if extra_mount.mount_destination is not None - else "" - ), + kernel_path=PurePosixPath(extra_mount.mount_destination) + if extra_mount.mount_destination + else None, ) for extra_mount in self.extra_mounts ] @@ -574,11 +572,9 @@ def to_model_revision_creator(self) -> ModelRevisionCreator: extra_mounts = [ MountInfo( vfolder_id=UUID(str(extra_mount.vfolder_id)), - kernel_path=PurePosixPath( - extra_mount.mount_destination - if extra_mount.mount_destination is not None - else "" - ), + kernel_path=PurePosixPath(extra_mount.mount_destination) + if extra_mount.mount_destination + else None, ) for extra_mount in self.extra_mounts ] diff --git a/src/ai/backend/manager/api/rest/deployment/adapter.py b/src/ai/backend/manager/api/rest/deployment/adapter.py index 0515dc79076..24cdaf58330 100644 --- a/src/ai/backend/manager/api/rest/deployment/adapter.py +++ b/src/ai/backend/manager/api/rest/deployment/adapter.py @@ -6,9 +6,7 @@ from __future__ import annotations -from datetime import UTC, datetime from pathlib import PurePosixPath -from typing import Any from uuid import UUID, uuid4 from ai.backend.common.data.model_deployment.types import DeploymentStrategy @@ -26,6 +24,7 @@ DeploymentFilter, DeploymentOrder, DeploymentPolicyDTO, + DeploymentStrategyInput, ModelMountConfigDTO, ModelRuntimeConfigDTO, NetworkConfigDTO, @@ -78,6 +77,7 @@ RouteTrafficStatus as ManagerRouteTrafficStatus, ) from ai.backend.manager.errors.api import InvalidAPIParameters +from ai.backend.manager.errors.deployment import IncompleteRevisionData from ai.backend.manager.models.deployment_policy import BlueGreenSpec, RollingUpdateSpec from ai.backend.manager.repositories.base import ( BatchQuerier, @@ -210,6 +210,9 @@ class RevisionAdapter(BaseFilterAdapter): def convert_to_dto(self, data: ModelRevisionData) -> RevisionDTO: """Convert ModelRevisionData to DTO.""" + mount_config = data.model_mount_config + if mount_config.vfolder_id is None: + raise IncompleteRevisionData(f"Revision {data.id} has incomplete model mount config") return RevisionDTO( id=data.id, name=data.name, @@ -225,11 +228,9 @@ def convert_to_dto(self, data: ModelRevisionData) -> RevisionDTO: runtime_variant=data.model_runtime_config.runtime_variant, ), model_mount_config=ModelMountConfigDTO( - # TODO: Generating a random UUID when vfolder_id is None creates a reference to a non-existent vfolder. Should raise an error instead. - vfolder_id=data.model_mount_config.vfolder_id or uuid4(), - # TODO: Empty string is not a valid path when mount_destination is None. Should make it a required field or assign a sensible default path. - mount_destination=data.model_mount_config.mount_destination or "", - definition_path=data.model_mount_config.definition_path, + vfolder_id=mount_config.vfolder_id, + mount_destination=mount_config.mount_destination, + definition_path=mount_config.definition_path, ), created_at=data.created_at, image_id=data.image_id, @@ -299,7 +300,7 @@ def convert_to_dto(self, data: RouteInfo) -> RouteDTO: session_id=str(data.session_id) if data.session_id else None, status=CommonRouteStatus(data.status.value), traffic_ratio=data.traffic_ratio, - created_at=data.created_at or datetime.now(tz=UTC), + created_at=data.created_at, revision_id=data.revision_id, traffic_status=CommonRouteTrafficStatus(data.traffic_status.value), error_data=data.error_data, @@ -390,8 +391,9 @@ def build_revision_creator(revision_input: RevisionInput) -> ModelRevisionCreato extra_mounts = [ MountInfo( vfolder_id=mount.vfolder_id, - # TODO: Empty string is not a valid path when mount_destination is None. Should make it a required field or assign a sensible default path. - kernel_path=PurePosixPath(mount.mount_destination or ""), + kernel_path=PurePosixPath(mount.mount_destination) + if mount.mount_destination + else None, ) for mount in revision_input.extra_mounts ] @@ -494,7 +496,7 @@ def build_creator( def _build_policy_config( self, - strategy_input: Any, # DeploymentStrategyInput + strategy_input: DeploymentStrategyInput, ) -> DeploymentPolicyConfig: """Build DeploymentPolicyConfig from strategy input.""" strategy = DeploymentStrategy(strategy_input.type) diff --git a/src/ai/backend/manager/data/deployment/types.py b/src/ai/backend/manager/data/deployment/types.py index 2b2d00caaf7..a121db9d2a2 100644 --- a/src/ai/backend/manager/data/deployment/types.py +++ b/src/ai/backend/manager/data/deployment/types.py @@ -244,7 +244,7 @@ class MountSpec: @dataclass class MountInfo: vfolder_id: UUID - kernel_path: PurePosixPath + kernel_path: PurePosixPath | None = None @dataclass @@ -388,7 +388,7 @@ class RouteInfo: session_id: SessionId | None status: RouteStatus traffic_ratio: float - created_at: datetime | None + created_at: datetime revision_id: UUID | None traffic_status: RouteTrafficStatus error_data: dict[str, Any] = field(default_factory=dict) @@ -521,7 +521,7 @@ class ModelDeploymentData: replica_state: ReplicaStateData default_deployment_strategy: DeploymentStrategy created_user_id: UUID - access_token_ids: UUID | None = None + access_token_ids: list[UUID] | None = None class DeploymentOrderField(enum.StrEnum): diff --git a/src/ai/backend/manager/data/model_serving/types.py b/src/ai/backend/manager/data/model_serving/types.py index 10047e4f50b..016df4e1c87 100644 --- a/src/ai/backend/manager/data/model_serving/types.py +++ b/src/ai/backend/manager/data/model_serving/types.py @@ -93,7 +93,7 @@ class RoutingData: session: uuid.UUID | None status: RouteStatus traffic_ratio: float - created_at: datetime | None + created_at: datetime error_data: dict[str, Any] diff --git a/src/ai/backend/manager/errors/deployment.py b/src/ai/backend/manager/errors/deployment.py index 5aa44050029..8ffaa0b05ab 100644 --- a/src/ai/backend/manager/errors/deployment.py +++ b/src/ai/backend/manager/errors/deployment.py @@ -145,3 +145,15 @@ def error_code(self) -> ErrorCode: operation=ErrorOperation.READ, error_detail=ErrorDetail.INVALID_PARAMETERS, ) + + +class IncompleteRevisionData(BackendAIError, web.HTTPInternalServerError): + error_type = "https://api.backend.ai/probs/incomplete-revision-data" + error_title = "Revision data is missing required fields." + + def error_code(self) -> ErrorCode: + return ErrorCode( + domain=ErrorDomain.MODEL_SERVICE, + operation=ErrorOperation.READ, + error_detail=ErrorDetail.INVALID_PARAMETERS, + ) diff --git a/src/ai/backend/manager/models/alembic/versions/b1009fe7f865_make_routings_created_at_non_nullable.py b/src/ai/backend/manager/models/alembic/versions/b1009fe7f865_make_routings_created_at_non_nullable.py new file mode 100644 index 00000000000..e4380a99aae --- /dev/null +++ b/src/ai/backend/manager/models/alembic/versions/b1009fe7f865_make_routings_created_at_non_nullable.py @@ -0,0 +1,38 @@ +"""Make routings.created_at non-nullable + +Revision ID: b1009fe7f865 +Revises: 0b1efbb2db84 +Create Date: 2026-03-06 04:11:09.336691 + +""" + +from alembic import op +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = "b1009fe7f865" +down_revision = "0b1efbb2db84" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # Fill any existing NULLs with now() before adding NOT NULL constraint + op.execute("UPDATE routings SET created_at = now() WHERE created_at IS NULL") + op.alter_column( + "routings", + "created_at", + existing_type=postgresql.TIMESTAMP(timezone=True), + nullable=False, + existing_server_default="now()", + ) + + +def downgrade() -> None: + op.alter_column( + "routings", + "created_at", + existing_type=postgresql.TIMESTAMP(timezone=True), + nullable=True, + existing_server_default="now()", + ) diff --git a/src/ai/backend/manager/models/routing/row.py b/src/ai/backend/manager/models/routing/row.py index 51a9d9c1f9f..d5db0d4a7bc 100644 --- a/src/ai/backend/manager/models/routing/row.py +++ b/src/ai/backend/manager/models/routing/row.py @@ -82,11 +82,11 @@ class RoutingRow(Base): # type: ignore[misc] ) weight: Mapped[int | None] = mapped_column("weight", sa.Integer(), nullable=True, default=None) traffic_ratio: Mapped[float] = mapped_column("traffic_ratio", sa.Float(), nullable=False) - created_at: Mapped[datetime | None] = mapped_column( + created_at: Mapped[datetime] = mapped_column( "created_at", sa.DateTime(timezone=True), server_default=sa.text("now()"), - nullable=True, + nullable=False, ) error_data: Mapped[dict[str, Any] | None] = mapped_column( diff --git a/src/ai/backend/manager/repositories/deployment/db_source/db_source.py b/src/ai/backend/manager/repositories/deployment/db_source/db_source.py index b2499f1106a..df2be03caa4 100644 --- a/src/ai/backend/manager/repositories/deployment/db_source/db_source.py +++ b/src/ai/backend/manager/repositories/deployment/db_source/db_source.py @@ -5,7 +5,7 @@ from collections.abc import AsyncIterator, Mapping, Sequence from contextlib import asynccontextmanager as actxmgr from dataclasses import dataclass -from datetime import UTC, datetime +from datetime import datetime from typing import Any, cast import sqlalchemy as sa @@ -882,7 +882,7 @@ async def get_routes_by_endpoint( session_id=SessionId(row.session) if row.session else None, status=row.status, traffic_ratio=row.traffic_ratio, - created_at=row.created_at or datetime.now(tz=UTC), + created_at=row.created_at, error_data=row.error_data or {}, ) for row in rows @@ -1447,7 +1447,7 @@ async def get_routes_by_statuses( session_id=SessionId(row.session) if row.session else None, status=row.status, traffic_ratio=row.traffic_ratio, - created_at=row.created_at or datetime.now(tz=UTC), + created_at=row.created_at, error_data=row.error_data or {}, ) route_data_list.append(route_data) diff --git a/src/ai/backend/manager/services/deployment/service.py b/src/ai/backend/manager/services/deployment/service.py index d37c107fd5b..4304afcc7d0 100644 --- a/src/ai/backend/manager/services/deployment/service.py +++ b/src/ai/backend/manager/services/deployment/service.py @@ -265,7 +265,7 @@ def _convert_route_info_to_replica_data(route: RouteInfo) -> ModelReplicaData: else ActivenessStatus.INACTIVE, weight=int(route.traffic_ratio * 100), # Convert ratio to weight detail=route.error_data, - created_at=route.created_at or datetime.now(tz=UTC), + created_at=route.created_at, )