Skip to content

feat: add TOS object store support via OpenDAL#7019

Merged
Xuanwo merged 1 commit into
lance-format:mainfrom
ddupg:codex/tos-opendal-release
Jun 4, 2026
Merged

feat: add TOS object store support via OpenDAL#7019
Xuanwo merged 1 commit into
lance-format:mainfrom
ddupg:codex/tos-opendal-release

Conversation

@ddupg

@ddupg ddupg commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

What changed

Add Volcengine TOS (tos://) object store support through OpenDAL.

  • Register a TOS object store provider for tos://bucket/path.
  • Add the tos feature to lance-io and enable it by default through rust/lance.
  • Support TOS_ / VOLCENGINE_ environment variables and tos_* storage options.
  • Document TOS configuration.

Testing

Validated against a real Volcengine TOS object store.

@github-actions github-actions Bot added A-python Python bindings A-java Java bindings + JNI labels Jun 1, 2026
@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

ACTION NEEDED
Lance follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

For details on the error please inspect the "PR Title Check" action.

@ddupg ddupg changed the title Add TOS object store support via OpenDAL feat: add TOS object store support via OpenDAL Jun 1, 2026
@github-actions github-actions Bot added the enhancement New feature or request label Jun 1, 2026
@codecov

codecov Bot commented Jun 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.40230% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-io/src/object_store/providers/tos.rs 95.37% 6 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@ddupg
ddupg force-pushed the codex/tos-opendal-release branch from 0796a4d to cf4e283 Compare June 1, 2026 07:41
@ddupg
ddupg marked this pull request as ready for review June 1, 2026 08:27

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@ddupg
ddupg force-pushed the codex/tos-opendal-release branch 2 times, most recently from a72a70e to a9f9678 Compare June 1, 2026 08:49
@ddupg

ddupg commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

@Xuanwo Could you please help review this? Appreciate it.

@yanghua

yanghua commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

@claude review

Comment thread rust/lance-io/src/object_store.rs Outdated

const DEFAULT_LOCAL_BLOCK_SIZE: usize = 4 * 1024; // 4KB block size
#[cfg(any(feature = "aws", feature = "gcp", feature = "azure"))]
#[cfg(any(feature = "aws", feature = "gcp", feature = "azure", feature = "tos"))]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 [Pre-existing] The #[cfg] gate for DEFAULT_CLOUD_BLOCK_SIZE at rust/lance-io/src/object_store.rs:64 only enables it for aws/gcp/azure/tos, but providers/oss.rs, providers/tencent.rs, and providers/huggingface.rs also import and use this constant. Building with --no-default-features --features oss (or just tencent or just huggingface) fails to compile with E0432: unresolved import crate::object_store::DEFAULT_CLOUD_BLOCK_SIZE. Since this PR touches this exact line to add tos, it'''s a good opportunity to backfill oss, tencent, and huggingface into the same gate.

Extended reasoning...

What the bug is

At rust/lance-io/src/object_store.rs:64, the constant DEFAULT_CLOUD_BLOCK_SIZE is defined under the cfg gate:

#[cfg(any(feature = "aws", feature = "gcp", feature = "azure", feature = "tos"))]
const DEFAULT_CLOUD_BLOCK_SIZE: usize = 64 * 1024;

However, three other provider modules also import and use this constant from their new_store implementations:

  • rust/lance-io/src/object_store/providers/oss.rs:14 (use crate::object_store::{..., DEFAULT_CLOUD_BLOCK_SIZE, ...}), used at line ~108 as params.block_size.unwrap_or(DEFAULT_CLOUD_BLOCK_SIZE).
  • rust/lance-io/src/object_store/providers/tencent.rs:12, used at line ~23.
  • rust/lance-io/src/object_store/providers/huggingface.rs:16, used at line ~179.

Each of these provider modules is itself gated on its own feature in providers.rs (#[cfg(feature = "oss")] pub mod oss; etc.).

Why existing code doesn'''t prevent it

In CI and default builds, aws, gcp, or azure is almost always enabled, which keeps DEFAULT_CLOUD_BLOCK_SIZE defined and masks the issue. The cfg gate intersection becomes vacuous: in practice, anyone using oss/tencent/huggingface is also using one of the gated features.

Step-by-step proof

  1. Edit rust/lance-io/Cargo.toml to remove the default of ["aws", "azure", "gcp"] or simply run with --no-default-features.
  2. Run: cargo check --no-default-features --features oss in rust/lance-io.
  3. Compilation fails with error[E0432]: unresolved import crate::object_store::DEFAULT_CLOUD_BLOCK_SIZE. The compiler points at providers/oss.rs:14 (the use site) and the cfg-disabled definition at object_store.rs:64.
  4. The same failure occurs with --features tencent and --features huggingface.

Impact

Anyone packaging Lance with a minimal feature set — for instance, only oss to support Alibaba Cloud users, or only huggingface for HF integrations — will hit a hard build failure. This is exactly the kind of compile-time matrix gap that the cfg system is meant to prevent.

How to fix

Extend the cfg list to include the other three opendal-based features that already use the constant:

#[cfg(any(
    feature = "aws",
    feature = "gcp",
    feature = "azure",
    feature = "oss",
    feature = "tencent",
    feature = "huggingface",
    feature = "tos",
))]
const DEFAULT_CLOUD_BLOCK_SIZE: usize = 64 * 1024;

Severity rationale

This is pre-existingoss, tencent, and huggingface were already omitted from the gate before this PR. The PR only added tos. However, since the PR touches this exact line and copies the same incomplete pattern (adding tos but missing the other three opendal providers), it'''s a natural opportunity to fix in passing.

@ddupg
ddupg force-pushed the codex/tos-opendal-release branch from a9f9678 to fa0c9c8 Compare June 2, 2026 10:59
@github-actions github-actions Bot added A-deps Dependency updates A-encoding Encoding, IO, file reader/writer A-docs Documentation labels Jun 2, 2026
@ddupg
ddupg force-pushed the codex/tos-opendal-release branch from b77cbaa to 6ea2998 Compare June 3, 2026 02:46

@Xuanwo Xuanwo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@Xuanwo
Xuanwo merged commit 2a73364 into lance-format:main Jun 4, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-deps Dependency updates A-docs Documentation A-encoding Encoding, IO, file reader/writer A-java Java bindings + JNI A-python Python bindings enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants