Skip to content

refactor(data): make dataviewer storage non-blocking - #1566

Draft
Alexandre Gattiker (algattik) wants to merge 1 commit into
mainfrom
algattik-issue-120-dataviewer-storage-refactor
Draft

Alexandre Gattiker (algattik) wants to merge 1 commit into
mainfrom
algattik-issue-120-dataviewer-storage-refactor

Conversation

@algattik

Copy link
Copy Markdown
Collaborator

Description

Moved blocking storage work off the FastAPI event loop, made Azure transfers asynchronous and resource-safe, and aligned storage adapter behavior. Fixed annotation serialization and save sequencing so language instructions persist after reload. Hardened the local launcher to detect failed child processes, preserve exit status, and reject occupied ports.

Closes #120

Type of Change

  • 🐛 Bug fix (non-breaking change fixing an issue)
  • ✨ New feature (non-breaking change adding functionality)
  • 💥 Breaking change (fix or feature causing existing functionality to change)
  • 📚 Documentation update
  • 🏗️ Infrastructure change (Terraform/IaC)
  • ♻️ Refactoring (no functional changes)

Component(s) Affected

  • infrastructure/terraform/prerequisites/ - Azure subscription setup
  • infrastructure/terraform/ - Terraform infrastructure
  • infrastructure/setup/ - OSMO control plane / Helm
  • workflows/ - Training and evaluation workflows
  • training/ - Training pipelines and scripts
  • docs/ - Documentation
  • data-management/viewer/ - Dataset analysis backend and frontend

Testing Performed

The manual plan covered deterministic local and Azure datasets, annotation CRUD and reload persistence, path validation, Hugging Face fallback, Azure discovery and synchronization, chunked upload/download and ranged streaming, event-loop responsiveness, SDK retry recovery/exhaustion, and browser annotation/export workflows.

Documentation Impact

  • No documentation changes needed
  • Documentation updated in this PR
  • Documentation issue filed

Bug Fix Checklist

  • Linked to issue being fixed
  • Regression test included, OR
  • Justification for no regression test:

Checklist

- 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
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 5b31842.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.95062% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.09%. Comparing base (8d49dca) to head (5b31842).

Files with missing lines Patch % Lines
...ent/viewer/backend/src/api/storage/blob_dataset.py 77.55% 5 Missing and 6 partials ⚠️
...management/viewer/backend/src/api/storage/azure.py 77.77% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1566      +/-   ##
==========================================
- Coverage   88.10%   88.09%   -0.02%     
==========================================
  Files         279      279              
  Lines       23203    23231      +28     
  Branches     3153     3203      +50     
==========================================
+ Hits        20444    20466      +22     
- Misses       2071     2075       +4     
- Partials      688      690       +2     
Flag Coverage Δ *Carryforward flag
go 100.00% <ø> (ø)
pester 86.64% <ø> (ø) Carriedforward from 8d49dca
pytest-data-pipeline 100.00% <ø> (ø) Carriedforward from 8d49dca
pytest-dataviewer 89.62% <80.00%> (-0.04%) ⬇️
pytest-dm-tools 100.00% <ø> (ø) Carriedforward from 8d49dca
pytest-evaluation 95.40% <ø> (ø)
pytest-fuzz 3.98% <0.00%> (-0.02%) ⬇️
pytest-inference 100.00% <ø> (ø) Carriedforward from 8d49dca
pytest-shared-ci 100.00% <ø> (ø) Carriedforward from 8d49dca
pytest-training 92.60% <ø> (ø) Carriedforward from 8d49dca
vitest 85.65% <100.00%> (-0.01%) ⬇️
vitest-app 85.65% <100.00%> (-0.01%) ⬇️
vitest-components 85.65% <100.00%> (-0.01%) ⬇️
vitest-features 85.65% <100.00%> (-0.01%) ⬇️
vitest-lib 85.65% <100.00%> (-0.01%) ⬇️
vitest-state 85.65% <100.00%> (-0.01%) ⬇️

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...ment/viewer/backend/src/api/storage/huggingface.py 93.70% <100.00%> (ø)
...management/viewer/backend/src/api/storage/local.py 98.79% <100.00%> (ø)
...ewer/frontend/src/hooks/use-annotation-workflow.ts 91.83% <100.00%> (ø)
...ement/viewer/frontend/src/hooks/use-annotations.ts 85.36% <100.00%> (ø)
...a-management/viewer/frontend/src/lib/api-client.ts 95.59% <100.00%> (+0.32%) ⬆️
...management/viewer/backend/src/api/storage/azure.py 93.96% <77.77%> (-1.53%) ⬇️
...ent/viewer/backend/src/api/storage/blob_dataset.py 87.08% <77.55%> (-0.11%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@algattik
Alexandre Gattiker (algattik) marked this pull request as draft September 18, 2026 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(dataviewer): fix async I/O, add retry policy, and unify storage adapter interfaces

2 participants