feat: Add Apache Gravitino virtual file system (gvfs://) read support in io module

[shaofengshi]

Changes Made

Add Gravitino virtual file system (gvfs://) support read support. So that user can use "gvfs://filesets/catalog/schema/fileset_name" path to access s3 (and other cloud storages) location.

This PR only implements the s3 as the first step. To ensure the functionality, the integration test cases are added with a MinIO service to simulate s3.

Related Issues

This is the second pr for #5503

[codecov]

Codecov Report

❌ Patch coverage is 67.66917% with 86 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.48%. Comparing base (535941d) to head (b104440).
⚠️ Report is 463 commits behind head on main.

Files with missing lines	Patch %	Lines
src/daft-io/src/gravitino.rs	63.44%	53 Missing ⚠️
src/common/io-config/src/python.rs	69.44%	22 Missing ⚠️
src/common/io-config/src/gravitino.rs	81.81%	6 Missing ⚠️
daft/filesystem.py	0.00%	2 Missing ⚠️
daft/gravitino/gravitino_catalog.py	50.00%	2 Missing ⚠️
src/daft-io/src/lib.rs	80.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #5766    +/-   ##
========================================
  Coverage   72.48%   72.48%            
========================================
  Files         966      968     +2     
  Lines      125838   126093   +255     
========================================
+ Hits        91211    91402   +191     
- Misses      34627    34691    +64     

Files with missing lines	Coverage Δ
daft/io/__init__.py	100.00% <ø> (ø)
src/common/io-config/src/config.rs	100.00% <100.00%> (ø)
src/common/io-config/src/lib.rs	53.12% <ø> (ø)
src/daft-io/src/lib.rs	77.01% <80.00%> (+0.82%)	⬆️
daft/filesystem.py	41.47% <0.00%> (+0.09%)	⬆️
daft/gravitino/gravitino_catalog.py	67.72% <50.00%> (+3.74%)	⬆️
src/common/io-config/src/gravitino.rs	81.81% <81.81%> (ø)
src/common/io-config/src/python.rs	45.00% <69.44%> (+2.04%)	⬆️
src/daft-io/src/gravitino.rs	63.44% <63.44%> (ø)

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[greptile-apps]

Greptile Overview

Greptile Summary

This PR implements Apache Gravitino virtual filesystem (gvfs://) read support, allowing Daft to access data through Gravitino's unified metadata catalog. The implementation follows Daft's existing filesystem integration patterns.

Key Changes:

• Adds gvfs://fileset/catalog/schema/fileset/path URL scheme with path parsing and validation in Rust
• Implements client caching in GravitinoSource to avoid redundant API calls for the same fileset
• Supports write operations through GravitinoOutputStream with in-memory buffering
• Includes comprehensive integration tests with MinIO for S3-backed filesets
• Adds to_io_config() method to GravitinoClient for easier configuration

Architecture:
The implementation bridges Python and Rust layers effectively. Python's GravitinoFileSystem wraps PyArrow filesystem interface, while Rust's GravitinoSource handles actual I/O by loading fileset metadata from Gravitino API, extracting storage credentials, and delegating to appropriate storage backends (S3, etc.).

Testing:
Excellent test coverage with 476 unit tests and comprehensive integration tests covering read, write, glob patterns, partitioned data, and error scenarios.

Confidence Score: 4/5

• This PR is safe to merge with minor style improvements recommended
• Score reflects solid implementation quality with comprehensive testing. The Rust code is well-tested with extensive unit tests (743 lines including tests). Integration tests properly validate S3 backend functionality. The only concerns are minor style issues in Python code (__getattr__ redundancy) that don't affect functionality.
• Pay closer attention to daft/io/gravitino_filesystem.py for the redundant __getattr__ implementation

Important Files Changed

File Analysis

Filename	Score	Overview
daft/io/gravitino_filesystem.py	4/5	Implements PyArrow filesystem wrapper for gvfs:// URLs with write support via buffering. The __getattr__ method returns dummy functions which could mask real issues.
src/daft-io/src/gravitino.rs	5/5	Well-tested Rust implementation with extensive path parsing validation and client caching. Comprehensive unit tests cover edge cases.
daft/gravitino/gravitino_catalog.py	5/5	Adds to_io_config() method to convert client configuration to IOConfig. Clean implementation with proper credential handling.
tests/integration/gravitino/test_gravitino_fileset_s3.py	5/5	Comprehensive integration tests with proper MinIO setup, covering read, write, glob, and partitioned data scenarios.
tests/io/test_gravitino_io.py	5/5	Thorough unit tests exercising Rust code paths through error scenarios. Tests URL validation, config validation, and client caching.

Sequence Diagram

sequenceDiagram
    participant User
    participant Daft
    participant Filesystem as filesystem.py
    participant GravFS as GravitinoFileSystem
    participant RustIO as Rust IO Layer
    participant GravSource as GravitinoSource
    participant GravAPI as Gravitino API
    participant S3

    User->>Daft: read_parquet("gvfs://fileset/cat/schema/fs/file.parquet")
    Daft->>Filesystem: _resolve_paths_and_filesystem()
    Filesystem->>Filesystem: get_protocol_from_path() → "gvfs"
    Filesystem->>Filesystem: _infer_filesystem()
    Filesystem->>GravFS: GravitinoFileSystem(io_config)
    GravFS-->>Filesystem: PyArrow FS wrapper
    Filesystem-->>Daft: filesystem + path
    
    Daft->>RustIO: io_glob() with gvfs:// path
    RustIO->>GravSource: get_or_create_io_client("cat.schema.fs")
    
    alt First access (not cached)
        GravSource->>GravAPI: load_fileset("cat.schema.fs")
        GravAPI-->>GravSource: fileset metadata + storage_location (s3://...)
        GravSource->>GravSource: Create IOClient with S3 credentials
        GravSource->>GravSource: Cache client for "cat.schema.fs"
    else Already cached
        GravSource->>GravSource: Return cached client
    end
    
    GravSource->>GravSource: fileset_path_to_source_and_url()
    GravSource->>GravSource: Parse gvfs:// → extract cat/schema/fs/path
    GravSource->>GravSource: Build S3 path: storage_location + path
    GravSource->>S3: get() via IOClient
    S3-->>GravSource: file data
    GravSource-->>RustIO: file data
    RustIO-->>Daft: DataFrame
    Daft-->>User: DataFrame

Loading

[greptile-apps]

Additional Comments (2)

1. daft/io/gravitino_filesystem.py, line 121-137 (link)

   style: __getattr__ returns dummy functions for missing attributes, which can mask legitimate AttributeErrors. Consider explicitly implementing only the methods PyArrow needs (like fileno, isatty, mode, name) instead of this catch-all approach.

2. daft/io/gravitino_filesystem.py, line 121-137 (link)

   style: __getattr__ creates redundant logic since explicit methods for fileno, isatty, mode, and name are already defined below (lines 210-220). The __getattr__ method will never be called for these attributes. Consider removing this method entirely or only handling truly dynamic attributes.

_{25 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[kevinzwang]

Aside from the comment about the GravatinoFileSystem, I am happy to merge these changes in. I am not so familiar with the nuances of gravatino catalogs, tables, and filesets but the changes seem fairly straightforward and isolated so I will trust that you have the logic implemented correctly. Thanks for the contribution!

[kevinzwang]

I'm not sure if GravatinoFileSystem is needed at all since we do not go through the PyArrow filesystem path for anything except certain write operations, and it looks like this PR only supports reads.

[shaofengshi]

Hi Kevin, you're righ, the GravatinoFileSystem was built to support gvfs:// writes. Previously I have implement both read and write function, and per your suggestion I split them into multiple PRs, this class was kept here. I think it can be removed now and added back in subsequent PR.

[shaofengshi]

I removed it from this pr; see the commit: eb35001

[shaofengshi]

also removed other unrelated codes, please review.

[kevinzwang]

I'm curious if writes can be implemented on the Rust side as well. That would be nice just so that it's in the same place. But for this PR, thanks for cleaning it up, will take another look!

[kevinzwang]

Thanks for the contribution! Re-running the failed tests and enabling auto-merge.

[shaofengshi]

Thanks for the contribution! Re-running the failed tests and enabling auto-merge.

This issue is still there: #5776
I run the iceberg integration test with the main branch, got the same. Not sure whether others also see this.

[kevinzwang]

@shaofengshi could you check if the gravatino Docker services are conflicting somehow with the Iceberg ones for the catalogs integration test? I'm not seeing the same errors on the main branch.

[shaofengshi]

@shaofengshi could you check if the gravatino Docker services are conflicting somehow with the Iceberg ones for the catalogs integration test? I'm not seeing the same errors on the main branch.

Thanks Kevin for the suggestion! There was a port conflict but as the integration test is executed in sequence (and iceberg is ahead of gravitino), so I'm not sure that is the root cause. Besides, just 1 iceberg test case failed (tests/integration/iceberg/test_iceberg_reads.py::TestIcebergCountPushdown::test_count_pushdown_with_delete_files[test_overlapping_deletes]), others are success, so I assume it is not environment issue.

Anyway I resolved the port conflict and other potential confliction in the docker service. Let's see how CI builds this time.

If the iceberg test failure couldn't be reproduced in your side, that might be my problem. I will dive into the details.

[shaofengshi]

Kevin, I have confirmed the iceberg integration test failure has no relationship with my PR. Xiaoxiaohu has reproduced that with main branch, and she is going to fix that with this PR: #5864

So, my PR is safe to merge. :)

[kevinzwang]

@shaofengshi I figured it out. It's because both the Iceberg and Gravitino Docker composes had a service named minio, and for some reason Docker compose will just override the existing service if a new one is created from a different compose file. I've renamed the service in your PR, which fixes the tests locally for me. Hopefully it works in CI as well.

[kevinzwang]

@shaofengshi looks like there's some style checks failing. Once you clean those up I'm happy to merge this in!

Note: you can run those checks locally by installing the pre-commit hooks via make hooks

[shaofengshi]

@shaofengshi looks like there's some style checks failing. Once you clean those up I'm happy to merge this in!

Note: you can run those checks locally by installing the pre-commit hooks via make hooks

Kevin, I have fixed the style issue. Please check now. @kevinzwang

[kevinzwang]

Looks great, thanks again for the contribution @shaofengshi! Merging it in.

A great next step would be to add information about the Gravitino integration to our docs as well!

[shaofengshi]

Looks great, thanks again for the contribution @shaofengshi! Merging it in.

A great next step would be to add information about the Gravitino integration to our docs as well!

Thank you Kevin! Sure, I will update the document in subsequent PR.