Resolve every Java-binding URL through the vortex-cloud registry, adding hf:// - #9273
Draft
robert3005 wants to merge 1 commit into
Draft
Resolve every Java-binding URL through the vortex-cloud registry, adding hf://#9273robert3005 wants to merge 1 commit into
robert3005 wants to merge 1 commit into
Conversation
Merging this PR will degrade performance by 11.21%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
robert3005
force-pushed
the
claude/vortex-jni-hf
branch
from
August 7, 2026 13:11
16e9e89 to
a0677fe
Compare
robert3005
force-pushed
the
claude/vortex-jni-hf
branch
from
August 7, 2026 14:03
a0677fe to
c320fe2
Compare
…g `hf://`
The Java binding built stores through its own scheme dispatch — bespoke S3,
Azure and GCS builders plus an authority-keyed cache — and keyed every read by
the full URL path. That assumption held for the schemes it served, but not for
`hf://`: a Hugging Face store is rooted at a repository and revision, which
occupy path segments, so a full-URL-path key would send the repository name to
the Hub as part of the file path. And the authority-keyed cache cannot serve
`hf://` at all, since every Hub repository shares the `datasets` authority.
Replace the dispatch with `vortex_cloud::Registry`, the same resolution the
Python and DuckDB bindings use. `make_object_store` reports the path of the URL
within the store it returns, and every caller (metadata reads, listing, deletes,
globbed data sources, the writer) keys by that. Caller properties are
`object_store` configuration keys already (`aws_access_key_id`, ...), so they
layer over the process environment into a per-property-set registry — stores
built with one caller's credentials must not serve another's requests.
The hardcoded S3 endpoint/path-style/allow-http and the Azure timeout survive
as defaults. A default must yield to *any* spelling of its key (`endpoint` and
`aws_endpoint` are one configuration), or both spellings reach the store
builder and whichever iterates last wins — so each default lists the spellings
that suppress it, and a test pins that the suppressed default is fully absent.
The OpenDAL-backed schemes keep a properties-native branch, since their
property names (`secret_id`, ...) are the services' own rather than environment
names. The crate's `opendal` feature flag is gone entirely: vortex-jni is an
unpublished cdylib built exactly one way, no Rust consumer exists to opt out,
and CI never exercised the off-combo, so the flag only added untested cfg
branches. The dependency is now unconditional and the shipped library serves
every scheme.
Verified with the crate's unit tests, the Java suite
(`./gradlew :vortex-jni:test`, 32 tests), and a live Hub read through
`DataSource.open("hf://datasets/...")` (10k rows, 1190 columns). The S3Mock
container test needs Docker and is left to CI; its requirements (http endpoint,
path-style) are what the retained S3 defaults preserve.
Towards #5379.
Signed-off-by: Robert Kruszewski <robert@spiraldb.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lc5zw7Le2T3pakDEUdKTYd
robert3005
force-pushed
the
claude/vortex-jni-hf
branch
from
August 7, 2026 16:07
c320fe2 to
8043b00
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
Companion to #9265, completing
hf://coverage across the bindings. The Java binding built stores through its own scheme dispatch — bespoke S3, Azure and GCS builders plus an authority-keyed cache — and keyed every read by the full URL path. That assumption held for the schemes it served, but not forhf://: a Hugging Face store is rooted at a repository and revision, which occupy path segments, so a full-URL-path key would send the repository name to the Hub as part of the file path. And the authority-keyed cache cannot servehf://at all, since every Hub repository shares thedatasetsauthority.Rather than special-casing
hf://, the whole dispatch is replaced withvortex_cloud::Registry— the same resolution the Python and DuckDB bindings use.What changes are included in this PR?
vortex-cloud:Registry::with_vars(the previously test-only fixed-variable constructor) is now public, so bindings whose callers pass per-request configuration can layer it over the process environment.vortex-jni:make_object_storeresolves through a per-property-setRegistryand reports the path of the URL within the store; every caller (metadata reads,listFiles, deletes, globbed data sources, the writer) keys by that instead of deriving a path from the URL. The bespoke S3/Azure/GCS builders and the authority-keyed store cache are gone.object_storeconfiguration keys already (aws_access_key_id,aws_endpoint, … — exactly whatHadoopUtils/VortexS3Propertiesemit), so they pass straight throughparse_url_opts. Distinct property sets get distinct registries, since a store built with one caller's credentials must not serve another's requests.allow_http) and the 120s Azure timeout survive as defaults that both the environment and properties override. A default yields to any spelling of its key —endpointandaws_endpointare one configuration, and if both reached the store builder, whichever iterates last would win — so each default lists the spellings that suppress it, and a test pins that a suppressed default is fully absent.opendalfeature flag is removed entirely:vortex-jniis an unpublished cdylib built exactly one way, no Rust consumer exists to opt out, and CI never exercised the off-combo, so the flag only added untested cfg branches.vortex-cloud/opendalis now an unconditional dependency and the shipped library servescos:///oss:///goosefs://out of the box. Those schemes keep a properties-native branch (their property names —secret_id, … — are the services' own, not environment names) with the authority-keyed cache that is sound for them.Verification
cargo test -p vortex-jni --lib— 7 passed; clippy clean;cargo test -p vortex-cloud --all-features— 71 passed../gradlew :vortex-jni:test— BUILD SUCCESSFUL, 32 JVM tests against the rebuilt native library.DataSource.open(session, "hf://datasets/danking00/statpopgen-benchmark/…gnomad….vortex")returned the exact row count (10,000) and schema (1,190 columns) in ~6s.VortexDataSourceS3MockTestneeds Docker (testcontainers), unavailable in this sandbox — left to CI, which is why this stays draft.Limitations mirror the other bindings: the Hub serves no listing, so wildcard globs and
listFilesfail with the store's listing error, while exact file paths resolve withheadalone; writes and deletes are rejected by the Hub.What APIs are changed? Are there any user-facing changes?
Java reads (
DataSource.open,NativeFiles) now accepthf://datasets/<owner>/<name>[@<revision>]/<path>URLs for exact file paths, and the shipped library serves the OpenDAL-backed schemes unconditionally (theopendalfeature flag no longer exists).vortex_cloud::Registry::with_varsis new public API. Two minor behavior shifts on the Rust side: environment variables can now override the previously hardcoded S3 defaults (properties still override everything), andmemory://URLs now resolve instead of erroring. No Java API changes.🤖 Generated with Claude Code
https://claude.ai/code/session_01Lc5zw7Le2T3pakDEUdKTYd