Skip to content

Commit 12db84f

Browse files
GWealecopybara-github
authored andcommitted
fix: Remove app name from FileArtifactService directory structure
Co-authored-by: George Weale <gweale@google.com> PiperOrigin-RevId: 834462462
1 parent dc3f60c commit 12db84f

File tree

2 files changed

+17
-53
lines changed

2 files changed

+17
-53
lines changed

src/google/adk/artifacts/file_artifact_service.py

Lines changed: 17 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -188,20 +188,18 @@ class FileArtifactService(BaseArtifactService):
188188

189189
# Storage layout matches the cloud and in-memory services:
190190
# root/
191-
# ├── apps/
192-
# │ └── {app_name}/
193-
# │ └── users/
194-
# │ └── {user_id}/
195-
# │ ├── sessions/
196-
# │ │ └── {session_id}/
197-
# │ │ └── artifacts/
198-
# │ │ └── {artifact_path}/ # derived from filename
199-
# │ │ └── versions/
200-
# │ │ └── {version}/
201-
# │ │ ├── {original_filename}
202-
# │ │ └── metadata.json
203-
# │ └── artifacts/
204-
# │ └── {artifact_path}/...
191+
# └── users/
192+
# └── {user_id}/
193+
# ├── sessions/
194+
# │ └── {session_id}/
195+
# │ └── artifacts/
196+
# │ └── {artifact_path}/ # derived from filename
197+
# │ └── versions/
198+
# │ └── {version}/
199+
# │ ├── {original_filename}
200+
# │ └── metadata.json
201+
# └── artifacts/
202+
# └── {artifact_path}/...
205203
#
206204
# Artifact paths are derived from the provided filenames: separators create
207205
# nested directories, and path traversal is rejected to keep the layout
@@ -217,19 +215,18 @@ def __init__(self, root_dir: Path | str):
217215
self.root_dir = Path(root_dir).expanduser().resolve()
218216
self.root_dir.mkdir(parents=True, exist_ok=True)
219217

220-
def _base_root(self, app_name: str, user_id: str) -> Path:
221-
"""Returns the artifacts root directory for an app/user combination."""
222-
return self.root_dir / "apps" / app_name / "users" / user_id
218+
def _base_root(self, user_id: str, /) -> Path:
219+
"""Returns the artifacts root directory for a user."""
220+
return self.root_dir / "users" / user_id
223221

224222
def _scope_root(
225223
self,
226-
app_name: str,
227224
user_id: str,
228225
session_id: Optional[str],
229226
filename: str,
230227
) -> Path:
231228
"""Returns the directory that represents the artifact scope."""
232-
base = self._base_root(app_name, user_id)
229+
base = self._base_root(user_id)
233230
if _is_user_scoped(session_id, filename):
234231
return _user_artifacts_dir(base)
235232
if not session_id:
@@ -240,14 +237,12 @@ def _scope_root(
240237

241238
def _artifact_dir(
242239
self,
243-
app_name: str,
244240
user_id: str,
245241
session_id: Optional[str],
246242
filename: str,
247243
) -> Path:
248244
"""Builds the directory path for an artifact."""
249245
scope_root = self._scope_root(
250-
app_name=app_name,
251246
user_id=user_id,
252247
session_id=session_id,
253248
filename=filename,
@@ -258,7 +253,6 @@ def _artifact_dir(
258253
def _build_artifact_version(
259254
self,
260255
*,
261-
app_name: str,
262256
user_id: str,
263257
session_id: Optional[str],
264258
filename: str,
@@ -270,7 +264,6 @@ def _build_artifact_version(
270264
metadata.canonical_uri
271265
if metadata and metadata.canonical_uri
272266
else self._canonical_uri(
273-
app_name=app_name,
274267
user_id=user_id,
275268
session_id=session_id,
276269
filename=filename,
@@ -289,15 +282,13 @@ def _build_artifact_version(
289282
def _canonical_uri(
290283
self,
291284
*,
292-
app_name: str,
293285
user_id: str,
294286
session_id: Optional[str],
295287
filename: str,
296288
version: int,
297289
) -> str:
298290
"""Builds the canonical file:// URI for an artifact payload."""
299291
artifact_dir = self._artifact_dir(
300-
app_name=app_name,
301292
user_id=user_id,
302293
session_id=session_id,
303294
filename=filename,
@@ -336,7 +327,6 @@ async def save_artifact(
336327
"""
337328
return await asyncio.to_thread(
338329
self._save_artifact_sync,
339-
app_name,
340330
user_id,
341331
filename,
342332
artifact,
@@ -346,7 +336,6 @@ async def save_artifact(
346336

347337
def _save_artifact_sync(
348338
self,
349-
app_name: str,
350339
user_id: str,
351340
filename: str,
352341
artifact: types.Part,
@@ -355,7 +344,6 @@ def _save_artifact_sync(
355344
) -> int:
356345
"""Saves an artifact to disk and returns its version."""
357346
artifact_dir = self._artifact_dir(
358-
app_name=app_name,
359347
user_id=user_id,
360348
session_id=session_id,
361349
filename=filename,
@@ -386,7 +374,6 @@ def _save_artifact_sync(
386374
raise ValueError("Artifact must have either inline_data or text content.")
387375

388376
canonical_uri = self._canonical_uri(
389-
app_name=app_name,
390377
user_id=user_id,
391378
session_id=session_id,
392379
filename=filename,
@@ -421,7 +408,6 @@ async def load_artifact(
421408
) -> Optional[types.Part]:
422409
return await asyncio.to_thread(
423410
self._load_artifact_sync,
424-
app_name,
425411
user_id,
426412
filename,
427413
session_id,
@@ -430,15 +416,13 @@ async def load_artifact(
430416

431417
def _load_artifact_sync(
432418
self,
433-
app_name: str,
434419
user_id: str,
435420
filename: str,
436421
session_id: Optional[str],
437422
version: Optional[int],
438423
) -> Optional[types.Part]:
439424
"""Loads an artifact from disk."""
440425
artifact_dir = self._artifact_dir(
441-
app_name=app_name,
442426
user_id=user_id,
443427
session_id=session_id,
444428
filename=filename,
@@ -493,21 +477,19 @@ async def list_artifact_keys(
493477
) -> list[str]:
494478
return await asyncio.to_thread(
495479
self._list_artifact_keys_sync,
496-
app_name,
497480
user_id,
498481
session_id,
499482
)
500483

501484
def _list_artifact_keys_sync(
502485
self,
503-
app_name: str,
504486
user_id: str,
505487
session_id: Optional[str],
506488
) -> list[str]:
507489
"""Lists artifact filenames for the given session/user."""
508490
filenames: set[str] = set()
509491

510-
base_root = self._base_root(app_name, user_id)
492+
base_root = self._base_root(user_id)
511493

512494
if session_id:
513495
session_root = _session_artifacts_dir(base_root, session_id)
@@ -550,21 +532,18 @@ async def delete_artifact(
550532
"""
551533
await asyncio.to_thread(
552534
self._delete_artifact_sync,
553-
app_name,
554535
user_id,
555536
filename,
556537
session_id,
557538
)
558539

559540
def _delete_artifact_sync(
560541
self,
561-
app_name: str,
562542
user_id: str,
563543
filename: str,
564544
session_id: Optional[str],
565545
) -> None:
566546
artifact_dir = self._artifact_dir(
567-
app_name=app_name,
568547
user_id=user_id,
569548
session_id=session_id,
570549
filename=filename,
@@ -585,21 +564,18 @@ async def list_versions(
585564
"""Lists all versions stored for an artifact."""
586565
return await asyncio.to_thread(
587566
self._list_versions_sync,
588-
app_name,
589567
user_id,
590568
filename,
591569
session_id,
592570
)
593571

594572
def _list_versions_sync(
595573
self,
596-
app_name: str,
597574
user_id: str,
598575
filename: str,
599576
session_id: Optional[str],
600577
) -> list[int]:
601578
artifact_dir = self._artifact_dir(
602-
app_name=app_name,
603579
user_id=user_id,
604580
session_id=session_id,
605581
filename=filename,
@@ -618,21 +594,18 @@ async def list_artifact_versions(
618594
"""Lists metadata for each artifact version on disk."""
619595
return await asyncio.to_thread(
620596
self._list_artifact_versions_sync,
621-
app_name,
622597
user_id,
623598
filename,
624599
session_id,
625600
)
626601

627602
def _list_artifact_versions_sync(
628603
self,
629-
app_name: str,
630604
user_id: str,
631605
filename: str,
632606
session_id: Optional[str],
633607
) -> list[ArtifactVersion]:
634608
artifact_dir = self._artifact_dir(
635-
app_name=app_name,
636609
user_id=user_id,
637610
session_id=session_id,
638611
filename=filename,
@@ -644,7 +617,6 @@ def _list_artifact_versions_sync(
644617
metadata = _read_metadata(metadata_path)
645618
artifact_versions.append(
646619
self._build_artifact_version(
647-
app_name=app_name,
648620
user_id=user_id,
649621
session_id=session_id,
650622
filename=filename,
@@ -667,7 +639,6 @@ async def get_artifact_version(
667639
"""Gets metadata for a specific artifact version."""
668640
return await asyncio.to_thread(
669641
self._get_artifact_version_sync,
670-
app_name,
671642
user_id,
672643
filename,
673644
session_id,
@@ -676,14 +647,12 @@ async def get_artifact_version(
676647

677648
def _get_artifact_version_sync(
678649
self,
679-
app_name: str,
680650
user_id: str,
681651
filename: str,
682652
session_id: Optional[str],
683653
version: Optional[int],
684654
) -> Optional[ArtifactVersion]:
685655
artifact_dir = self._artifact_dir(
686-
app_name=app_name,
687656
user_id=user_id,
688657
session_id=session_id,
689658
filename=filename,
@@ -701,7 +670,6 @@ def _get_artifact_version_sync(
701670
metadata_path = _metadata_path(artifact_dir, version_to_read)
702671
metadata = _read_metadata(metadata_path)
703672
return self._build_artifact_version(
704-
app_name=app_name,
705673
user_id=user_id,
706674
session_id=session_id,
707675
filename=filename,

tests/unittests/artifacts/test_artifact_service.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -614,8 +614,6 @@ async def test_file_metadata_camelcase(tmp_path, artifact_service_factory):
614614
metadata_path = (
615615
tmp_path
616616
/ "artifacts"
617-
/ "apps"
618-
/ "myapp"
619617
/ "users"
620618
/ "user123"
621619
/ "sessions"
@@ -677,8 +675,6 @@ async def test_file_list_artifact_versions(tmp_path, artifact_service_factory):
677675
version_payload_path = (
678676
tmp_path
679677
/ "artifacts"
680-
/ "apps"
681-
/ "myapp"
682678
/ "users"
683679
/ "user123"
684680
/ "sessions"

0 commit comments

Comments
 (0)