Resolve every Java-binding URL through the vortex-cloud registry, adding hf:// - #9273
Open
robert3005 wants to merge 5 commits into
Open
Resolve every Java-binding URL through the vortex-cloud registry, adding hf://#9273robert3005 wants to merge 5 commits into
robert3005 wants to merge 5 commits into
Conversation
Merging this PR will degrade performance by 9.27%
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
3 times, most recently
from
August 8, 2026 22:10
8043b00 to
dfabcd8
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 old builders' hardcoded configuration (generic S3 endpoint, path-style,
allow-http, a 120s Azure timeout) is gone with them: stores are configured by
the environment and the caller's properties alone, as in the other bindings.
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.
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 10, 2026 00:13
dfabcd8 to
091f481
Compare
Signed-off-by: Robert Kruszewski <github@robertk.io>
robert3005
marked this pull request as ready for review
August 10, 2026 01:06
…p merge The develop merge resolved the `vortex-cloud` dependency line to the base branch's `optional = true` spelling. With the `opendal` feature section gone, nothing enables an optional dependency, so the crate was never linked and the whole workspace failed to compile vortex-jni. 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
…p:// The removed JNI store builders hardcoded `allow_http`, which the registry no longer applies. `object_store` rejects `http://` endpoints unless allowed, so map an explicitly plain-HTTP `fs.s3a.endpoint` (LocalStack, MinIO, S3Mock) onto `aws_allow_http` where the rest of the S3A configuration is translated. 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
A property overrides the environment by exact key. CI runners export AWS_REGION-style variables, so a short spelling (`region`) races its environment alias at the store builder instead of overriding it — reproducible locally with `AWS_REGION=x cargo test -p vortex-jni`. 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
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.
Remove special handling of object stores from vortex-jni and use upstream logic from vortex-cloud