feat: Add video upload and list_objects functionality to ObjectStore #1195

getter111 · 2025-11-20T03:44:34Z

Description

This PR adds video upload functionality to NAT, enabling multimodal workflows that require video processing. It integrates with NAT's existing object store infrastructure and includes a companion UI in the nat-ui submodule.

Key Changes

Backend:

Object Store Interface: Added list_objects() method to ObjectStore base class with implementation in S3ObjectStore (supports AWS S3 and MinIO). Returns metadata-only listings with optional prefix filtering and pagination
FastAPI Video Routes: New endpoints (POST /videos, GET /videos, DELETE /videos/{video_key}) for video upload, listing, and deletion with UUID-based naming (videos/{uuid}/{filename})
Testing: Added 20+ integration tests in test_video_upload_routes.py covering upload/list/delete operations with MinIO fixture
Documentation: Updated object-store.md with video upload examples and configuration patterns

Frontend:

Updated nat-ui submodule with video library components (see related PR below)

NAT.UI.Demo.mp4

Related PRs

This PR depends on NVIDIA/NeMo-Agent-Toolkit-UI#62

Closes

By Submitting this PR I confirm:

I am familiar with the Contributing Guidelines.
We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
- Any contribution which contains commits that are not Signed-Off will not be accepted.
When the PR is ready for review, new or existing tests cover these changes.
When the PR is ready for review, the documentation is up to date with these changes.

Summary by CodeRabbit

Release Notes

New Features
- Object listing with optional prefix-based filtering and metadata retrieval without downloading full content
- Video upload API with endpoints for uploading, listing, and managing videos
Documentation
- Updated with examples of object listing and video management workflows
Tests
- Added comprehensive tests for object listing and video upload functionality

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Backend: - ObjectStoreListItem model (metadata only, no data) - S3ObjectStore with pagination and path-style addressing - InMemoryObjectStore for testing - FastAPI video routes using videos/ prefix organization - MinIO test fixture and 20 tests - Updated documentation with examples Frontend (nat-ui submodule): - VideoLibrary, VideoUpload, VideoLibraryModal components - videoUpload API client proxying through backend - Settings integration and route constants Signed-off-by: Alex Lin <[email protected]>

…tore

copy-pr-bot · 2025-11-20T03:44:38Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

coderabbitai · 2025-11-20T03:44:50Z

Walkthrough

The changes introduce a new list_objects() method to the ObjectStore interface for memory-efficient, prefix-filtered metadata retrieval returning ObjectStoreListItem objects (excluding data content). Implementations are added to in-memory and S3-based stores. Video upload endpoints (POST/GET/DELETE) are added to FastAPI for video management. Supporting tests, test infrastructure for MinIO integration, and documentation are included.

Changes

Cohort / File(s)	Summary
Object Store Model & Interface `src/nat/object_store/models.py`, `src/nat/object_store/interfaces.py`	New `ObjectStoreListItem(BaseModel)` with fields: key, size, content_type, metadata, last_modified (no data payload). New abstract method `list_objects(prefix: str \| None = None) -> list[ObjectStoreListItem]` added to ObjectStore interface.
Object Store Implementations `src/nat/object_store/in_memory_object_store.py`, `packages/nvidia_nat_s3/src/nat/plugins/s3/s3_object_store.py`	Implemented `list_objects()` for in-memory store (filters by prefix, skips directory keys, returns metadata). S3 implementation uses paginator with prefix filtering, fetches per-object metadata via head_object, handles ClientError, supports path-style addressing for non-AWS endpoints.
FastAPI Video Upload Routes `src/nat/front_ends/fastapi/fastapi_front_end_plugin_worker.py`	Added `add_video_upload_route()` method with POST (upload with metadata), GET (list by prefix), DELETE (idempotent) endpoints under /videos. Handles UUID-based path generation, filename sanitization, content-type validation, conflict resolution (409), and metadata propagation.
Test Infrastructure `packages/nvidia_nat_s3/tests/conftest.py`	Session-scoped `minio_server()` fixture with port detection (`is_port_open()` helper) for MinIO integration tests; provides bucket and connection credentials.
Object Store Tests `packages/nvidia_nat_test/src/nat/test/object_store_tests.py`	New `test_list_objects()` method covering various prefixes, empty results, metadata validation (keys, content_type, size), and cleanup.
Video Upload Route Tests `tests/nat/front_ends/fastapi/test_video_upload_routes.py`	Integration tests for video endpoints: upload validation, content-type filtering, list enumeration, idempotent deletion, and end-to-end workflows using in-memory store.
Documentation `docs/source/store-and-retrieve/object-store.md`	Added ObjectStoreListItem description, `list_objects()` usage examples with prefix filtering, File Server and Video Upload Integration sections with HTTP endpoints, expanded Error Handling (NoSuchKeyError).
External Submodule `external/nat-ui`	Submodule reference updated from commit 8b91a7e... to 04efa5b...; no API or behavior changes.

Sequence Diagrams

sequenceDiagram
    participant Client
    participant FastAPI
    participant ObjectStore
    participant S3

    rect rgb(200, 220, 255)
    Note over Client,S3: list_objects() Flow
    Client->>FastAPI: GET /videos?prefix=videos/
    FastAPI->>ObjectStore: list_objects(prefix="videos/")
    ObjectStore->>S3: list_objects_v2(Prefix="videos/")
    S3-->>ObjectStore: [object_keys]
    loop Per object
        ObjectStore->>S3: head_object(key)
        S3-->>ObjectStore: {size, content_type, metadata, last_modified}
    end
    ObjectStore-->>FastAPI: [ObjectStoreListItem, ...]
    FastAPI-->>Client: [{key, size, content_type, ...}, ...]
    end

    rect rgb(220, 255, 220)
    Note over Client,S3: Video Upload Flow
    Client->>FastAPI: POST /videos (file + metadata)
    FastAPI->>FastAPI: Validate content_type
    FastAPI->>FastAPI: Sanitize filename, generate UUID
    FastAPI->>ObjectStore: put_object(key="videos/{uuid}/{filename}", data, metadata)
    ObjectStore->>S3: put_object(key, data, metadata)
    S3-->>ObjectStore: Success
    ObjectStore-->>FastAPI: Success
    FastAPI-->>Client: {video_key, filename, content_type, size, uuid}
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

S3 implementation: Paginator logic with per-object metadata fetching via head_object; error handling and path-style addressing configuration require careful validation.
FastAPI video endpoints: Filename sanitization, UUID path generation, content-type validation, and metadata propagation need verification for security (path traversal) and correctness.
Test fixtures: MinIO integration setup with port detection; verify fixture skips gracefully when service unavailable.
Consistency across stores: Verify all ObjectStore implementations (in-memory, S3) return consistent ObjectStoreListItem structure and handle prefix filtering identically.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 63.33% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately captures both main features added in this PR: video upload functionality and the list_objects method for ObjectStore, uses imperative mood, and is concise at 68 characters.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

packages/nvidia_nat_s3/tests/conftest.py (1)
31-55: LGTM! The MinIO fixture is well-designed.

The minio_server fixture provides good developer experience:

Automatically skips tests when MinIO isn't running

Comprehensive docstring with docker commands for setup

Session scope is appropriate for this shared resource

Clear skip message guides developers to start MinIO

Consider adding a return type hint to improve type safety:
 @pytest.fixture(scope="session")
-def minio_server():
+def minio_server() -> dict[str, str]:
This is a minor enhancement and fully optional.
src/nat/front_ends/fastapi/fastapi_front_end_plugin_worker.py (1)
751-778: Simplify redundant exception handling.

The delete operation correctly implements:

Security validation (key must start with videos/)

Idempotent behavior (deleting non-existent videos returns success)

Proper error handling with appropriate status codes

However, there's redundant exception handling with two consecutive except NoSuchKeyError blocks (lines 760-762 and 765-767) that perform the same action.

Simplify by removing the redundant catch block:
         try:
             try:
                 await object_store_client.delete_object(video_key)
             except NoSuchKeyError:
                 logger.info(f"Video {video_key} not found during delete - treating as success (idempotent)")
-                return {"message": "Video deleted successfully (was already deleted)", "video_key": video_key}
+            return {"message": "Video deleted successfully", "video_key": video_key}
-            return {"message": "Video deleted successfully", "video_key": video_key}
-        except NoSuchKeyError as e:
-            logger.info(f"Video {video_key} not found: {e}")
-            return {"message": "Video deleted successfully (was already deleted)", "video_key": video_key}
         except HTTPException:
             raise
         except Exception as e:
This maintains the same idempotent behavior with clearer logic flow.
docs/source/store-and-retrieve/object-store.md (1)

253-253: Consider adding example output to console command blocks. Markdownlint (MD014) flags lines 253, 257, and 261 as showing commands without output. For consistency with documentation conventions and to match similar patterns, consider either: (1) adding example response output below each command, or (2) removing the $ prompt to format as plain code blocks. Note: Similar curl examples at lines 210, 214, 218, 222 use the same format, so this may be a false positive or style inconsistency that could be addressed project-wide.

Also applies to: 257-257, 261-261

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b00007e and 90f666c.

📒 Files selected for processing (10)

docs/source/store-and-retrieve/object-store.md (4 hunks)
external/nat-ui (1 hunks)
packages/nvidia_nat_s3/src/nat/plugins/s3/s3_object_store.py (3 hunks)
packages/nvidia_nat_s3/tests/conftest.py (1 hunks)
packages/nvidia_nat_test/src/nat/test/object_store_tests.py (2 hunks)
src/nat/front_ends/fastapi/fastapi_front_end_plugin_worker.py (4 hunks)
src/nat/object_store/in_memory_object_store.py (2 hunks)
src/nat/object_store/interfaces.py (2 hunks)
src/nat/object_store/models.py (2 hunks)
tests/nat/front_ends/fastapi/test_video_upload_routes.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (5)

**/*