Skip to content

Commit b338b63

Browse files
jopemachineclaude
andcommitted
fix(BA-5827): apply review feedback on Fragment DTOs and actions
- Drop the FK detail from `AppConfigFragmentKeyInput.name` description — the FK is documented at the SQL / model layer; the DTO description should not leak that detail. - Add `scope_type` filter to `AppConfigFragmentFilter` (was missing even though scope_type is part of the natural key) plus the matching `AppConfigFragmentConditions.by_scope_type_equals` factory. - Add `UPDATED_AT` to `AppConfigFragmentOrderField` and the `AppConfigFragmentOrders.updated_at` factory; wire it into the adapter's order mapping. - `AppConfigFragmentAction.entity_id()` now returns the row id (or None when the row id is not yet known at action time), not the natural key. Applies to get / create / update / purge action + result pairs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 879a5ef commit b338b63

9 files changed

Lines changed: 42 additions & 15 deletions

File tree

src/ai/backend/common/dto/manager/v2/app_config_fragment/request.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from __future__ import annotations
66

77
from typing import Any
8+
from uuid import UUID
89

910
from pydantic import Field
1011

@@ -39,7 +40,7 @@ class AppConfigFragmentKeyInput(BaseRequestModel):
3940
name: str = Field(
4041
min_length=1,
4142
max_length=128,
42-
description="Policy name (FK to `app_config_policies.config_name`).",
43+
description="Policy name.",
4344
)
4445

4546

@@ -73,7 +74,12 @@ class PurgeAppConfigFragmentInput(BaseRequestModel):
7374
class AppConfigFragmentFilter(BaseRequestModel):
7475
"""Filter for app-config fragment search."""
7576

77+
id_in: list[UUID] | None = Field(
78+
default=None,
79+
description="Match rows whose `id` is in this list (used by Relay node loaders).",
80+
)
7681
name: StringFilter | None = Field(default=None, description="Filter by policy name.")
82+
scope_type: AppConfigScopeType | None = Field(default=None, description="Filter by scope_type.")
7783
scope_id: StringFilter | None = Field(default=None, description="Filter by scope_id.")
7884

7985

src/ai/backend/common/dto/manager/v2/app_config_fragment/types.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,4 @@ class AppConfigFragmentOrderField(StrEnum):
3131
SCOPE_ID = "scope_id"
3232
NAME = "name"
3333
CREATED_AT = "created_at"
34+
UPDATED_AT = "updated_at"

src/ai/backend/manager/api/adapters/app_config_fragment.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,8 @@ def _build_querier_from_input(self, input: SearchAppConfigFragmentsInput) -> Bat
186186

187187
def _convert_filter(self, filter: AppConfigFragmentFilter) -> list[QueryCondition]:
188188
conditions: list[QueryCondition] = []
189+
if filter.id_in is not None:
190+
conditions.append(AppConfigFragmentConditions.by_ids(filter.id_in))
189191
if filter.name is not None:
190192
condition = self.convert_string_filter(
191193
filter.name,
@@ -197,6 +199,10 @@ def _convert_filter(self, filter: AppConfigFragmentFilter) -> list[QueryConditio
197199
)
198200
if condition is not None:
199201
conditions.append(condition)
202+
if filter.scope_type is not None:
203+
conditions.append(
204+
AppConfigFragmentConditions.by_scope_type_equals(filter.scope_type.value)
205+
)
200206
if filter.scope_id is not None:
201207
condition = self.convert_string_filter(
202208
filter.scope_id,
@@ -224,6 +230,8 @@ def _convert_orders(orders: list[AppConfigFragmentOrder]) -> list[QueryOrder]:
224230
result.append(AppConfigFragmentOrders.name(ascending))
225231
case "created_at":
226232
result.append(AppConfigFragmentOrders.created_at(ascending))
233+
case "updated_at":
234+
result.append(AppConfigFragmentOrders.updated_at(ascending))
227235
return result
228236

229237
@staticmethod

src/ai/backend/manager/models/app_config_fragment/conditions.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,13 @@ def inner() -> sa.ColumnElement[bool]:
131131

132132
by_scope_id_in = staticmethod(make_string_in_factory(AppConfigFragmentRow.scope_id))
133133

134+
@staticmethod
135+
def by_scope_type_equals(scope_type: str) -> QueryCondition:
136+
def inner() -> sa.ColumnElement[bool]:
137+
return AppConfigFragmentRow.scope_type == scope_type
138+
139+
return inner
140+
134141
@staticmethod
135142
def by_cursor_forward(cursor_id: str) -> QueryCondition:
136143
cursor_uuid = uuid.UUID(cursor_id)

src/ai/backend/manager/models/app_config_fragment/orders.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,9 @@ def created_at(ascending: bool = True) -> QueryOrder:
3232
if ascending:
3333
return AppConfigFragmentRow.created_at.asc()
3434
return AppConfigFragmentRow.created_at.desc()
35+
36+
@staticmethod
37+
def updated_at(ascending: bool = True) -> QueryOrder:
38+
if ascending:
39+
return AppConfigFragmentRow.updated_at.asc()
40+
return AppConfigFragmentRow.updated_at.desc()

src/ai/backend/manager/services/app_config_fragment/actions/create.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ class CreateAppConfigFragmentAction(AppConfigFragmentAction):
1818

1919
@override
2020
def entity_id(self) -> str | None:
21-
return f"{self.key.scope_type}:{self.key.scope_id}:{self.key.name}"
21+
# Row id is not known at action time (insert hasn't happened yet).
22+
return None
2223

2324
@override
2425
@classmethod
@@ -32,6 +33,4 @@ class CreateAppConfigFragmentActionResult(BaseActionResult):
3233

3334
@override
3435
def entity_id(self) -> str | None:
35-
return (
36-
f"{self.fragment.key.scope_type}:{self.fragment.key.scope_id}:{self.fragment.key.name}"
37-
)
36+
return str(self.fragment.id)

src/ai/backend/manager/services/app_config_fragment/actions/get.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ class GetAppConfigFragmentAction(AppConfigFragmentAction):
1616

1717
@override
1818
def entity_id(self) -> str | None:
19-
return f"{self.key.scope_type}:{self.key.scope_id}:{self.key.name}"
19+
# Row id is not known at action time (lookup is by natural key).
20+
return None
2021

2122
@override
2223
@classmethod
@@ -32,6 +33,4 @@ class GetAppConfigFragmentActionResult(BaseActionResult):
3233
def entity_id(self) -> str | None:
3334
if self.fragment is None:
3435
return None
35-
return (
36-
f"{self.fragment.key.scope_type}:{self.fragment.key.scope_id}:{self.fragment.key.name}"
37-
)
36+
return str(self.fragment.id)

src/ai/backend/manager/services/app_config_fragment/actions/purge.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ class PurgeAppConfigFragmentAction(AppConfigFragmentAction):
1313

1414
@override
1515
def entity_id(self) -> str | None:
16-
return f"{self.key.scope_type}:{self.key.scope_id}:{self.key.name}"
16+
# Row id is not known at action time (lookup is by natural key).
17+
return None
1718

1819
@override
1920
@classmethod
@@ -28,4 +29,5 @@ class PurgeAppConfigFragmentActionResult(BaseActionResult):
2829

2930
@override
3031
def entity_id(self) -> str | None:
31-
return f"{self.key.scope_type}:{self.key.scope_id}:{self.key.name}"
32+
# Row is gone post-purge; no surviving id to surface.
33+
return None

src/ai/backend/manager/services/app_config_fragment/actions/update.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ class UpdateAppConfigFragmentAction(AppConfigFragmentAction):
1818

1919
@override
2020
def entity_id(self) -> str | None:
21-
return f"{self.key.scope_type}:{self.key.scope_id}:{self.key.name}"
21+
# Row id is not known at action time (lookup is by natural key).
22+
return None
2223

2324
@override
2425
@classmethod
@@ -34,6 +35,4 @@ class UpdateAppConfigFragmentActionResult(BaseActionResult):
3435
def entity_id(self) -> str | None:
3536
if self.fragment is None:
3637
return None
37-
return (
38-
f"{self.fragment.key.scope_type}:{self.fragment.key.scope_id}:{self.fragment.key.name}"
39-
)
38+
return str(self.fragment.id)

0 commit comments

Comments
 (0)