Skip to content

Commit 5b31842

Browse files
committed
refactor(data): make storage adapters non-blocking
- move storage I/O off the event loop and unify adapter behavior - persist annotation payloads across frontend and backend boundaries - use mirrored dependency locks and harden local service startup Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1b95c18-4d4e-42e3-9d45-873695d8de2f ♻️ - Generated by Copilot
1 parent 8d49dca commit 5b31842

9 files changed

Lines changed: 198 additions & 79 deletions

File tree

data-management/viewer/backend/src/api/storage/azure.py

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,10 @@ def __init__(
8181
self.sas_token = sas_token
8282
self.use_managed_identity = use_managed_identity
8383
self._client: BlobServiceClient | None = None
84+
self._credential: DefaultAzureCredential | None = None
8485

8586
async def _get_client(self) -> BlobServiceClient:
86-
"""Get or create the blob service client."""
87+
"""Get or create the client with the SDK's default exponential retry policy."""
8788
if self._client is None:
8889
account_url = f"https://{self.account_name}.blob.core.windows.net"
8990

@@ -93,10 +94,10 @@ async def _get_client(self) -> BlobServiceClient:
9394
credential=self.sas_token,
9495
)
9596
else:
96-
credential = DefaultAzureCredential()
97+
self._credential = DefaultAzureCredential()
9798
self._client = BlobServiceClient(
9899
account_url=account_url,
99-
credential=credential,
100+
credential=self._credential,
100101
)
101102

102103
return self._client
@@ -254,7 +255,12 @@ async def delete_annotation(self, dataset_id: str, episode_index: int) -> bool:
254255
raise StorageError(f"Failed to delete blob {blob_path}: {e}", cause=e)
255256

256257
async def close(self) -> None:
257-
"""Close the blob service client."""
258-
if self._client is not None:
259-
await self._client.close()
260-
self._client = None
258+
"""Close the blob service client and managed credential."""
259+
client, self._client = self._client, None
260+
credential, self._credential = self._credential, None
261+
try:
262+
if client is not None:
263+
await client.close()
264+
finally:
265+
if credential is not None:
266+
await credential.close()

data-management/viewer/backend/src/api/storage/blob_dataset.py

Lines changed: 61 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,16 @@
1818

1919
from __future__ import annotations
2020

21+
import asyncio
2122
import json
2223
import logging
2324
from collections.abc import AsyncIterator
2425
from pathlib import Path
2526
from typing import TYPE_CHECKING
2627

28+
import aiofiles
29+
import pyarrow.parquet as pq
30+
2731
if TYPE_CHECKING:
2832
pass
2933

@@ -87,6 +91,7 @@ def __init__(
8791
self.container_name = container_name
8892
self.sas_token = sas_token
8993
self._client: BlobServiceClient | None = None
94+
self._credential: AsyncDefaultAzureCredential | None = None
9095
self._info_cache: dict[str, dict] = {}
9196
# Per-dataset cache of episode_index -> {camera -> (chunk, file, from_ts, to_ts)}
9297
self._episode_video_cache: dict[str, dict[int, dict[str, tuple[int, int, float, float]]]] = {}
@@ -101,7 +106,7 @@ def get_blob_prefix(dataset_id: str) -> str:
101106
return dataset_id_to_blob_prefix(dataset_id)
102107

103108
async def _get_client(self) -> BlobServiceClient:
104-
"""Return a lazily-initialized async BlobServiceClient."""
109+
"""Return a client with the SDK's default exponential retry policy."""
105110
if self._client is None:
106111
account_url = f"https://{self.account_name}.blob.core.windows.net"
107112
if self.sas_token:
@@ -110,10 +115,10 @@ async def _get_client(self) -> BlobServiceClient:
110115
credential=self.sas_token,
111116
)
112117
else:
113-
credential = AsyncDefaultAzureCredential()
118+
self._credential = AsyncDefaultAzureCredential()
114119
self._client = BlobServiceClient(
115120
account_url=account_url,
116-
credential=credential,
121+
credential=self._credential,
117122
)
118123
return self._client
119124

@@ -411,12 +416,7 @@ async def _load_episode_video_metadata(
411416
dataset_id: str,
412417
) -> dict[int, dict[str, tuple[int, int, float, float]]] | None:
413418
"""Download and parse meta/episodes/chunk-*/file-*.parquet for video lookup."""
414-
try:
415-
import io
416-
417-
import pyarrow.parquet as pq
418-
except ImportError:
419-
return None
419+
import io
420420

421421
prefix = self.get_blob_prefix(dataset_id)
422422
meta_prefix = f"{prefix}/meta/episodes/"
@@ -431,7 +431,7 @@ async def _load_episode_video_metadata(
431431
data = await self._read_blob_bytes(blob.name)
432432
if data is None:
433433
continue
434-
table = pq.read_table(io.BytesIO(data))
434+
table = await asyncio.to_thread(pq.read_table, io.BytesIO(data))
435435
cols = table.column_names
436436
if "episode_index" not in cols:
437437
continue
@@ -553,12 +553,14 @@ async def upload_video(self, dataset_id: str, camera: str, episode_idx: int, loc
553553
"""Upload a locally generated video to blob storage.
554554
555555
Creates a dedicated client to avoid event loop conflicts when called
556-
from a worker thread via asyncio.new_event_loop().
556+
from a worker thread via asyncio.new_event_loop(). The client uses the
557+
SDK's default exponential retry policy.
557558
"""
558559
prefix = self.get_blob_prefix(dataset_id)
559560
blob_path = f"{prefix}/meta/videos/{camera}/episode_{episode_idx:06d}.mp4"
560561

561562
account_url = f"https://{self.account_name}.blob.core.windows.net"
563+
credential = None
562564
try:
563565
credential = AsyncDefaultAzureCredential() if not self.sas_token else None
564566
effective_credential = self.sas_token or credential
@@ -567,17 +569,23 @@ async def upload_video(self, dataset_id: str, camera: str, episode_idx: int, loc
567569
async with client:
568570
container = client.get_container_client(self.container_name)
569571
blob_client = container.get_blob_client(blob_path)
570-
with open(local_path, "rb") as f:
571-
await blob_client.upload_blob(f, overwrite=True)
572-
573-
if credential:
574-
await credential.close()
572+
await blob_client.upload_blob(self._read_file_chunks(local_path), overwrite=True)
575573

576574
logger.info("Uploaded video to blob: %s", blob_path)
577575
return True
578576
except Exception as e:
579577
logger.warning("Failed to upload video to blob '%s': %s", blob_path, e)
580578
return False
579+
finally:
580+
if credential is not None:
581+
await credential.close()
582+
583+
@staticmethod
584+
async def _read_file_chunks(path: Path, chunk_size: int = 1024 * 1024) -> AsyncIterator[bytes]:
585+
"""Read a local file asynchronously in bounded chunks."""
586+
async with aiofiles.open(path, "rb") as file:
587+
while chunk := await file.read(chunk_size):
588+
yield chunk
581589

582590
# ------------------------------------------------------------------
583591
# Parquet / metadata sync to local temp dir (enables existing loaders)
@@ -598,7 +606,7 @@ async def sync_dataset_to_local(self, dataset_id: str, local_dir: Path) -> bool:
598606
Returns:
599607
True if sync completed successfully, False on critical failure.
600608
"""
601-
local_dir.mkdir(parents=True, exist_ok=True)
609+
await asyncio.to_thread(local_dir.mkdir, parents=True, exist_ok=True)
602610

603611
try:
604612
client = await self._get_client()
@@ -621,14 +629,14 @@ async def sync_dataset_to_local(self, dataset_id: str, local_dir: Path) -> bool:
621629
continue
622630

623631
local_path = local_dir / relative
624-
local_path.parent.mkdir(parents=True, exist_ok=True)
632+
await asyncio.to_thread(local_path.parent.mkdir, parents=True, exist_ok=True)
625633

626-
if local_path.exists():
634+
if await asyncio.to_thread(local_path.exists):
627635
continue # Already synced
628636

629637
data = await self._read_blob_bytes(blob.name)
630638
if data is not None:
631-
local_path.write_bytes(data)
639+
await asyncio.to_thread(local_path.write_bytes, data)
632640
synced_count += 1
633641

634642
logger.info(
@@ -668,7 +676,7 @@ async def sync_meta_only_to_local(self, dataset_id: str, local_dir: Path) -> boo
668676
Returns:
669677
True if meta/info.json was successfully downloaded, False otherwise.
670678
"""
671-
local_dir.mkdir(parents=True, exist_ok=True)
679+
await asyncio.to_thread(local_dir.mkdir, parents=True, exist_ok=True)
672680

673681
try:
674682
client = await self._get_client()
@@ -685,17 +693,17 @@ async def sync_meta_only_to_local(self, dataset_id: str, local_dir: Path) -> boo
685693
continue
686694

687695
local_path = local_dir / relative
688-
local_path.parent.mkdir(parents=True, exist_ok=True)
696+
await asyncio.to_thread(local_path.parent.mkdir, parents=True, exist_ok=True)
689697

690-
if local_path.exists():
698+
if await asyncio.to_thread(local_path.exists):
691699
continue
692700

693701
data = await self._read_blob_bytes(blob.name)
694702
if data is not None:
695-
local_path.write_bytes(data)
703+
await asyncio.to_thread(local_path.write_bytes, data)
696704

697705
info_path = local_dir / "meta" / "info.json"
698-
if not info_path.exists():
706+
if not await asyncio.to_thread(info_path.exists):
699707
logger.warning(
700708
"meta/info.json not found for dataset '%s'",
701709
dataset_id.replace("\r", "").replace("\n", ""),
@@ -725,7 +733,7 @@ async def sync_hdf5_dataset_to_local(self, dataset_id: str, local_dir: Path) ->
725733
downloading full episode data. Episode HDF5 files are fetched
726734
on-demand via sync_hdf5_episode_to_local.
727735
"""
728-
local_dir.mkdir(parents=True, exist_ok=True)
736+
await asyncio.to_thread(local_dir.mkdir, parents=True, exist_ok=True)
729737
prefix = self.get_blob_prefix(dataset_id)
730738
try:
731739
client = await self._get_client()
@@ -735,30 +743,30 @@ async def sync_hdf5_dataset_to_local(self, dataset_id: str, local_dir: Path) ->
735743
if blob.name.endswith(".json"):
736744
filename = blob.name.rsplit("/", 1)[-1]
737745
local_path = local_dir / filename
738-
if local_path.exists():
746+
if await asyncio.to_thread(local_path.exists):
739747
continue
740748
data = await self._read_blob_bytes(blob.name)
741749
if data is not None:
742-
local_path.write_bytes(data)
750+
await asyncio.to_thread(local_path.write_bytes, data)
743751
elif blob.name.endswith(".hdf5"):
744752
found_hdf5 = True
745753
filename = blob.name.rsplit("/", 1)[-1]
746754
local_path = local_dir / filename
747-
if not local_path.exists():
748-
local_path.touch()
755+
if not await asyncio.to_thread(local_path.exists):
756+
await asyncio.to_thread(local_path.touch)
749757
elif blob.name.endswith(".mp4") and "/meta/videos/" in blob.name:
750758
relative = blob.name[len(prefix + "/") :]
751759
local_path = local_dir / relative
752-
if local_path.exists():
760+
if await asyncio.to_thread(local_path.exists):
753761
continue
754-
local_path.parent.mkdir(parents=True, exist_ok=True)
762+
await asyncio.to_thread(local_path.parent.mkdir, parents=True, exist_ok=True)
755763
blob_client = container.get_blob_client(blob.name)
756764
download = await blob_client.download_blob()
757765
tmp_path = local_path.with_suffix(".mp4.tmp")
758-
with open(tmp_path, "wb") as f:
766+
async with aiofiles.open(tmp_path, "wb") as file:
759767
async for chunk in download.chunks():
760-
f.write(chunk)
761-
tmp_path.rename(local_path)
768+
await file.write(chunk)
769+
await asyncio.to_thread(tmp_path.replace, local_path)
762770
logger.info("Downloaded cached video: %s", relative)
763771
return found_hdf5
764772
except Exception as e:
@@ -791,17 +799,17 @@ async def sync_hdf5_episode_to_local(self, dataset_id: str, local_dir: Path, epi
791799
if filename not in patterns:
792800
continue
793801
local_path = local_dir / filename
794-
if local_path.exists() and local_path.stat().st_size > 0:
802+
if await asyncio.to_thread(self._is_non_empty_file, local_path):
795803
return True
796804
blob_client = container.get_blob_client(blob.name)
797805
download = await blob_client.download_blob()
798806
tmp_path = local_path.with_suffix(".hdf5.tmp")
799807
written = 0
800-
with open(tmp_path, "wb") as f:
808+
async with aiofiles.open(tmp_path, "wb") as file:
801809
async for chunk in download.chunks():
802-
f.write(chunk)
810+
await file.write(chunk)
803811
written += len(chunk)
804-
tmp_path.rename(local_path)
812+
await asyncio.to_thread(tmp_path.replace, local_path)
805813
logger.info(
806814
"Downloaded HDF5 episode %d for '%s' (%d bytes)",
807815
episode_idx,
@@ -819,6 +827,11 @@ async def sync_hdf5_episode_to_local(self, dataset_id: str, local_dir: Path, epi
819827
)
820828
return False
821829

830+
@staticmethod
831+
def _is_non_empty_file(path: Path) -> bool:
832+
"""Return whether a path exists and contains data."""
833+
return path.exists() and path.stat().st_size > 0
834+
822835
async def get_hdf5_dataset_config(self, dataset_id: str) -> dict | None:
823836
"""Read dataset_config.json for a dataset."""
824837
data = await self._read_blob_bytes(f"{self.get_blob_prefix(dataset_id)}/dataset_config.json")
@@ -853,7 +866,12 @@ async def count_hdf5_episodes(self, dataset_id: str) -> int:
853866
# ------------------------------------------------------------------
854867

855868
async def close(self) -> None:
856-
"""Release the internal BlobServiceClient."""
857-
if self._client is not None:
858-
await self._client.close()
859-
self._client = None
869+
"""Release the internal BlobServiceClient and managed credential."""
870+
client, self._client = self._client, None
871+
credential, self._credential = self._credential, None
872+
try:
873+
if client is not None:
874+
await client.close()
875+
finally:
876+
if credential is not None:
877+
await credential.close()

data-management/viewer/backend/src/api/storage/huggingface.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,10 +174,10 @@ async def list_episodes(self) -> list[EpisodeMeta]:
174174

175175
try:
176176
# Check for episode parquet files
177-
chunk_dirs = fs.ls(episodes_dir)
177+
chunk_dirs = await asyncio.to_thread(fs.ls, episodes_dir)
178178
for chunk_dir in chunk_dirs:
179179
if "chunk-" in chunk_dir:
180-
episode_files = fs.ls(chunk_dir)
180+
episode_files = await asyncio.to_thread(fs.ls, chunk_dir)
181181
for ep_file in episode_files:
182182
if ep_file.endswith(".parquet"):
183183
# Extract episode index from filename

data-management/viewer/backend/src/api/storage/local.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ async def get_annotation(self, dataset_id: str, episode_index: int) -> EpisodeAn
6666
Returns:
6767
EpisodeAnnotationFile if annotations exist, None otherwise.
6868
"""
69-
file_path = self._get_annotation_path(dataset_id, episode_index)
69+
file_path = await asyncio.to_thread(self._get_annotation_path, dataset_id, episode_index)
7070

7171
try:
7272
if not await aiofiles.os.path.exists(file_path):
@@ -96,8 +96,8 @@ async def save_annotation(self, dataset_id: str, episode_index: int, annotation:
9696
Raises:
9797
StorageError: If the save operation fails.
9898
"""
99-
file_path = self._get_annotation_path(dataset_id, episode_index)
100-
annotations_dir = self._get_annotations_dir(dataset_id)
99+
annotations_dir = await asyncio.to_thread(self._get_annotations_dir, dataset_id)
100+
file_path = annotations_dir / f"episode_{episode_index:06d}.json"
101101

102102
try:
103103
# Ensure directory exists
@@ -145,7 +145,7 @@ async def list_annotated_episodes(self, dataset_id: str) -> list[int]:
145145
Returns:
146146
Sorted list of episode indices that have annotations.
147147
"""
148-
annotations_dir = self._get_annotations_dir(dataset_id)
148+
annotations_dir = await asyncio.to_thread(self._get_annotations_dir, dataset_id)
149149

150150
try:
151151
if not await aiofiles.os.path.exists(annotations_dir):
@@ -177,7 +177,7 @@ async def delete_annotation(self, dataset_id: str, episode_index: int) -> bool:
177177
Returns:
178178
True if annotations were deleted, False if they didn't exist.
179179
"""
180-
file_path = self._get_annotation_path(dataset_id, episode_index)
180+
file_path = await asyncio.to_thread(self._get_annotation_path, dataset_id, episode_index)
181181

182182
try:
183183
if not await aiofiles.os.path.exists(file_path):

data-management/viewer/frontend/src/hooks/use-annotation-workflow.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export function useAnnotationWorkflow(
7777
if (!currentAnnotation || !currentDatasetId) return
7878

7979
try {
80-
saveMutation.save()
80+
await saveMutation.save()
8181
markSaved()
8282
onSaveSuccess?.()
8383
} catch (error) {

data-management/viewer/frontend/src/hooks/use-annotations.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,12 +145,12 @@ export function useSaveCurrentAnnotation() {
145145
const currentAnnotation = useAnnotationStore((state) => state.currentAnnotation)
146146
const mutation = useSaveAnnotation()
147147

148-
const save = () => {
148+
const save = async (): Promise<void> => {
149149
if (!currentDataset || currentIndex < 0 || !currentAnnotation) {
150150
return
151151
}
152152

153-
mutation.mutate({
153+
await mutation.mutateAsync({
154154
datasetId: currentDataset.id,
155155
episodeIndex: currentIndex,
156156
annotation: currentAnnotation,

0 commit comments

Comments
 (0)