Skip to content

Commit b06cdbd

Browse files
committed
Address PR review: fix existing-group save, auth, and race
- Frontend sends groupName (not groupId UUID) as targetName so mode=existing resolves to the right registry directory. - Remove silent exception swallow in _compute_next_version so transient ls() errors no longer fall through to v1 and overwrite existing versions. - Add auth dependencies to save_model_to_registry to match the dataset endpoint. - Eagerly create the destination vN directory as a reservation to close the TOCTOU race between version computation and the background copy; return 409 on collision.
1 parent e7bc1b8 commit b06cdbd

3 files changed

Lines changed: 30 additions & 11 deletions

File tree

api/transformerlab/routers/experiment/jobs.py

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1529,15 +1529,12 @@ async def _compute_next_version(group_dir: str) -> str:
15291529

15301530
highest = 0
15311531
if await storage.exists(group_dir):
1532-
try:
1533-
entries = await storage.ls(group_dir, detail=False)
1534-
for entry in entries:
1535-
name = entry.rstrip("/").split("/")[-1]
1536-
match = re.match(r"^v(\d+)$", name)
1537-
if match:
1538-
highest = max(highest, int(match.group(1)))
1539-
except Exception:
1540-
pass
1532+
entries = await storage.ls(group_dir, detail=False)
1533+
for entry in entries:
1534+
name = entry.rstrip("/").split("/")[-1]
1535+
match = re.match(r"^v(\d+)$", name)
1536+
if match:
1537+
highest = max(highest, int(match.group(1)))
15411538
return f"v{highest + 1}"
15421539

15431540

@@ -1597,6 +1594,16 @@ async def save_dataset_to_registry(
15971594
dest_path = storage.join(datasets_registry_dir, group_name, version_label)
15981595
asset_id = f"{group_name}/{version_label}"
15991596

1597+
# Eagerly create the destination directory as a reservation to prevent
1598+
# concurrent saves from computing the same version label (TOCTOU).
1599+
try:
1600+
await storage.makedirs(dest_path, exist_ok=False)
1601+
except FileExistsError:
1602+
raise HTTPException(
1603+
status_code=409,
1604+
detail=f"Version '{version_label}' for group '{group_name}' already exists — please retry",
1605+
)
1606+
16001607
asyncio.create_task(
16011608
_save_dataset_to_registry(
16021609
job_id=job_id,
@@ -1659,6 +1666,8 @@ async def save_model_to_registry(
16591666
mode: str = Query("new", description="'new' to create a new entry, 'existing' to add version to existing group"),
16601667
tag: str = Query("latest", description="Tag to assign to the new version"),
16611668
description: Optional[str] = Query(None, description="Human-readable description for the version"),
1669+
user_and_team=Depends(get_user_and_team),
1670+
session: AsyncSession = Depends(get_async_session),
16621671
):
16631672
"""Copy a model from job's models directory to the global models registry."""
16641673

@@ -1704,6 +1713,16 @@ async def save_model_to_registry(
17041713
dest_path = storage.join(models_registry_dir, group_name, version_label)
17051714
asset_id = f"{group_name}/{version_label}"
17061715

1716+
# Eagerly create the destination directory as a reservation to prevent
1717+
# concurrent saves from computing the same version label (TOCTOU).
1718+
try:
1719+
await storage.makedirs(dest_path, exist_ok=False)
1720+
except FileExistsError:
1721+
raise HTTPException(
1722+
status_code=409,
1723+
detail=f"Version '{version_label}' for group '{group_name}' already exists — please retry",
1724+
)
1725+
17071726
asyncio.create_task(
17081727
_save_model_to_registry(
17091728
job_id=job_id,

src/renderer/components/Experiment/Tasks/JobArtifacts/DatasetsSection.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export default function DatasetsSection({
119119
experimentId: experimentInfo?.id,
120120
jobId: jobId.toString(),
121121
datasetName,
122-
targetName: info.groupId || info.groupName,
122+
targetName: info.groupName,
123123
mode: info.mode,
124124
tag: info.tag,
125125
description: info.description,

src/renderer/components/Experiment/Tasks/JobArtifacts/ModelsSection.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export default function ModelsSection({
119119
experimentId: experimentInfo?.id,
120120
jobId: jobId.toString(),
121121
modelName,
122-
targetName: info.groupId || info.groupName,
122+
targetName: info.groupName,
123123
mode: info.mode,
124124
tag: info.tag,
125125
description: info.description,

0 commit comments

Comments
 (0)