diff --git a/java/vortex-spark/src/main/java/dev/vortex/spark/config/HadoopUtils.java b/java/vortex-spark/src/main/java/dev/vortex/spark/config/HadoopUtils.java index d091269df0a..2f3c4d78d90 100644 --- a/java/vortex-spark/src/main/java/dev/vortex/spark/config/HadoopUtils.java +++ b/java/vortex-spark/src/main/java/dev/vortex/spark/config/HadoopUtils.java @@ -35,6 +35,11 @@ public static Map s3PropertiesFromHadoopConf(Configuration hadoo qualified = "https://" + qualified; } properties.setEndpoint(qualified); + // object_store rejects plain-HTTP endpoints (LocalStack, MinIO, S3Mock) + // unless explicitly allowed. + if (qualified.startsWith("http://")) { + properties.setAllowHttp(true); + } break; case FS_S3A_ENDPOINT_REGION: properties.setRegion(entry.getValue()); diff --git a/java/vortex-spark/src/main/java/dev/vortex/spark/config/VortexS3Properties.java b/java/vortex-spark/src/main/java/dev/vortex/spark/config/VortexS3Properties.java index f8298891f18..2bd2221e413 100644 --- a/java/vortex-spark/src/main/java/dev/vortex/spark/config/VortexS3Properties.java +++ b/java/vortex-spark/src/main/java/dev/vortex/spark/config/VortexS3Properties.java @@ -14,6 +14,7 @@ public final class VortexS3Properties { private static final String SESSION_TOKEN = "aws_session_token"; private static final String REGION = "aws_region"; private static final String ENDPOINT = "aws_endpoint"; + private static final String ALLOW_HTTP = "aws_allow_http"; private static final String SKIP_SIGNATURE = "aws_skip_signature"; private final Map properties = Maps.newHashMap(); @@ -62,6 +63,10 @@ public void setEndpoint(String endpoint) { properties.put(ENDPOINT, endpoint); } + public void setAllowHttp(boolean allowHttp) { + properties.put(ALLOW_HTTP, Boolean.toString(allowHttp)); + } + public void setSkipSignature(boolean skipSignature) { properties.put(SKIP_SIGNATURE, Boolean.toString(skipSignature)); } diff --git a/java/vortex-spark/src/test/java/dev/vortex/spark/config/HadoopUtilsTest.java b/java/vortex-spark/src/test/java/dev/vortex/spark/config/HadoopUtilsTest.java index 8cd10202da0..234db1a5ed6 100644 --- a/java/vortex-spark/src/test/java/dev/vortex/spark/config/HadoopUtilsTest.java +++ b/java/vortex-spark/src/test/java/dev/vortex/spark/config/HadoopUtilsTest.java @@ -5,6 +5,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Map; @@ -61,15 +62,15 @@ void s3QualifiesBareEndpoint() { void s3PreservesSchemedEndpoint() { Configuration httpConf = emptyConf(); httpConf.set(HadoopUtils.FS_S3A_ENDPOINT, "http://localhost:9000"); - assertEquals( - "http://localhost:9000", - HadoopUtils.s3PropertiesFromHadoopConf(httpConf).get("aws_endpoint")); + Map httpProperties = HadoopUtils.s3PropertiesFromHadoopConf(httpConf); + assertEquals("http://localhost:9000", httpProperties.get("aws_endpoint")); + assertEquals("true", httpProperties.get("aws_allow_http")); Configuration httpsConf = emptyConf(); httpsConf.set(HadoopUtils.FS_S3A_ENDPOINT, "https://s3.example.com"); - assertEquals( - "https://s3.example.com", - HadoopUtils.s3PropertiesFromHadoopConf(httpsConf).get("aws_endpoint")); + Map httpsProperties = HadoopUtils.s3PropertiesFromHadoopConf(httpsConf); + assertEquals("https://s3.example.com", httpsProperties.get("aws_endpoint")); + assertNull(httpsProperties.get("aws_allow_http")); } @Test diff --git a/vortex-cloud/src/registry/mod.rs b/vortex-cloud/src/registry/mod.rs index 51a4042d7ac..b854569959a 100644 --- a/vortex-cloud/src/registry/mod.rs +++ b/vortex-cloud/src/registry/mod.rs @@ -86,17 +86,12 @@ pub struct Registry { } /// Source of the configuration variables consulted when building a store. -/// -/// Tests construct a registry over a fixed set of variables rather than mutating the process -/// environment, which is unsound when tests run on multiple threads within one process (the -/// `std::env::set_var` block became `unsafe` in Rust 2024 for exactly this reason). #[derive(Debug, Default)] enum EnvSource { /// Read from the process environment. #[default] Process, /// A fixed set of variables. - #[cfg(test)] Fixed(Vec<(String, String)>), } @@ -106,7 +101,6 @@ impl EnvSource { fn lookup(&self, key: &str) -> Option { match self { EnvSource::Process => std::env::var(key).ok(), - #[cfg(test)] EnvSource::Fixed(vars) => vars .iter() .find(|(k, _)| k.eq_ignore_ascii_case(key)) @@ -120,7 +114,6 @@ impl EnvSource { EnvSource::Process => std::env::vars() .map(|(k, v)| (k.to_ascii_lowercase(), v)) .collect(), - #[cfg(test)] EnvSource::Fixed(vars) => vars .iter() .map(|(k, v)| (k.to_ascii_lowercase(), v.clone())) @@ -177,9 +170,12 @@ impl Registry { Self::default() } - /// Create a registry over a fixed set of configuration variables. - #[cfg(test)] - fn with_env(vars: I) -> Self + /// Create a registry over a fixed set of configuration variables, consulted instead of the + /// process environment when building stores. + /// + /// Lookups are case-insensitive. Pass each key at most once — the set's consumers disagree + /// on which duplicate wins. + pub fn with_vars(vars: I) -> Self where I: IntoIterator, { diff --git a/vortex-cloud/src/registry/tests.rs b/vortex-cloud/src/registry/tests.rs index 79dc931b28d..ef0931b05fa 100644 --- a/vortex-cloud/src/registry/tests.rs +++ b/vortex-cloud/src/registry/tests.rs @@ -14,7 +14,7 @@ use super::Registry; /// A registry whose S3 configuration comes from a fixed map rather than the process environment, /// so these tests neither read nor mutate global state. fn registry() -> Registry { - Registry::with_env([("AWS_REGION".to_string(), "us-east-3".to_string())]) + Registry::with_vars([("AWS_REGION".to_string(), "us-east-3".to_string())]) } /// A percent-encoded segment (as HuggingFace dataset URLs use for `refs/convert/parquet`) decodes diff --git a/vortex-jni/Cargo.toml b/vortex-jni/Cargo.toml index 648c1a79e4f..f2da780be75 100644 --- a/vortex-jni/Cargo.toml +++ b/vortex-jni/Cargo.toml @@ -33,18 +33,13 @@ tracing-subscriber = { workspace = true, features = ["env-filter"] } url = { workspace = true } vortex = { workspace = true, features = ["object_store", "files"] } vortex-arrow = { workspace = true } -vortex-cloud = { workspace = true, optional = true } +vortex-cloud = { workspace = true, features = ["hf", "opendal", "registry"] } vortex-parquet-variant = { workspace = true } vortex-spatial = { workspace = true } [dev-dependencies] jni = { workspace = true, features = ["invocation"] } -[features] -# Enable OpenDAL-backed object stores (Tencent COS, Alibaba OSS) for `cos://` and `oss://` URLs. -# This pulls in the `opendal` dependency, so it is opt-in. -opendal = ["vortex-cloud/opendal"] - [lib] crate-type = ["cdylib"] diff --git a/vortex-jni/src/data_source.rs b/vortex-jni/src/data_source.rs index 516a0aa7f55..7e50b0d934b 100644 --- a/vortex-jni/src/data_source.rs +++ b/vortex-jni/src/data_source.rs @@ -30,7 +30,6 @@ use vortex::io::filesystem::FileSystemRef; use vortex::io::runtime::BlockingRuntime; use vortex::io::session::RuntimeSessionExt; use vortex::scan::DataSourceRef; -use vortex::utils::aliases::hash_map::HashMap; use vortex_arrow::ArrowSessionExt; use crate::RUNTIME; @@ -93,23 +92,11 @@ pub extern "system" fn Java_dev_vortex_jni_NativeDataSource_open( .map(|g| parse_uri_or_path(g.as_str())) .collect::>()?; - let mut fs_cache: HashMap = HashMap::new(); - for glob_url in &glob_urls { - let base = base_url(glob_url); - if !fs_cache.contains_key(&base) { - let fs = object_store_fs(glob_url, &properties, session.handle())?; - fs_cache.insert(base, fs); - } - } - + // Glob by the path the resolver reports — only it knows how deep each store is mounted. let mut builder = MultiFileDataSource::new(session.clone()); for glob_url in &glob_urls { - let base = base_url(glob_url); - let fs = fs_cache - .get(&base) - .cloned() - .unwrap_or_else(|| unreachable!("fs cached for every base url")); - builder = builder.with_glob(glob_url.path(), Some(fs)); + let (fs, glob) = object_store_fs(glob_url, &properties, session.handle())?; + builder = builder.with_glob(glob, Some(fs)); } let inner = RUNTIME @@ -190,13 +177,6 @@ pub extern "system" fn Java_dev_vortex_jni_NativeDataSource_openFiles( }) } -/// URL with the path cleared, used as a cache key for filesystem reuse. -fn base_url(url: &Url) -> Url { - let mut base = url.clone(); - base.set_path(""); - base -} - #[unsafe(no_mangle)] pub extern "system" fn Java_dev_vortex_jni_NativeDataSource_free( _env: EnvUnowned, @@ -274,17 +254,3 @@ pub extern "system" fn Java_dev_vortex_jni_NativeDataSource_byteSize( Ok(()) }); } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_base_url_strips_path() { - let url = Url::parse("s3://bucket/a/b/c").unwrap(); - let base = base_url(&url); - assert_eq!(base.scheme(), "s3"); - assert_eq!(base.host_str(), Some("bucket")); - assert_eq!(base.path(), ""); - } -} diff --git a/vortex-jni/src/file.rs b/vortex-jni/src/file.rs index 1044ed7dc01..5369f9b965b 100644 --- a/vortex-jni/src/file.rs +++ b/vortex-jni/src/file.rs @@ -16,7 +16,6 @@ use jni::objects::JObjectArray; use jni::objects::JString; use jni::sys::jlong; use jni::sys::jobject; -use object_store::path::Path; use vortex::buffer::ByteBuffer; use vortex::error::VortexResult; use vortex::error::vortex_err; @@ -159,11 +158,7 @@ pub extern "system" fn Java_dev_vortex_jni_NativeFiles_readMetadata( let url = parse_uri_or_path(&uri)?; let properties = extract_properties(env, &options)?; - let fs = object_store_fs(&url, &properties, session.handle())?; - // `FileSystem` keys are literal, already-decoded paths, so decode as `listFiles` does. - let path = Path::from_url_path(url.path()) - .map_err(|_| vortex_err!("cannot parse uri as object_store Path"))? - .to_string(); + let (fs, path) = object_store_fs(&url, &properties, session.handle())?; let source = RUNTIME.block_on(async { fs.open_read(&path).await })?; let segments = read_metadata_segments(session, source, &path, None)?; @@ -226,11 +221,9 @@ pub extern "system" fn Java_dev_vortex_jni_NativeFiles_listFiles( let properties = extract_properties(env, &options)?; - let fs = object_store_fs(&url, &properties, session.handle())?; - let prefix = Path::from_url_path(url.path()) - .map_err(|_| vortex_err!("cannot parse root_path as object_store Path"))?; + let (fs, prefix) = object_store_fs(&url, &properties, session.handle())?; - let mut stream = fs.list(prefix.as_ref()); + let mut stream = fs.list(&prefix); let paths_vec = RUNTIME.block_on(async move { let mut paths = Vec::new(); @@ -287,7 +280,7 @@ pub extern "system" fn Java_dev_vortex_jni_NativeFiles_delete( let properties = extract_properties(env, &options)?; - let fs = object_store_fs(&store_url, &properties, session.handle())?; + let (fs, _path) = object_store_fs(&store_url, &properties, session.handle())?; RUNTIME.block_on(async { for uri in delete_uris { diff --git a/vortex-jni/src/object_store.rs b/vortex-jni/src/object_store.rs index a2534fccda0..24d9c80ed1f 100644 --- a/vortex-jni/src/object_store.rs +++ b/vortex-jni/src/object_store.rs @@ -1,182 +1,108 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors -use std::str::FromStr; use std::sync::Arc; use std::sync::LazyLock; -use std::time::Duration; -use object_store::ClientOptions; use object_store::ObjectStore; -use object_store::ObjectStoreScheme; -use object_store::aws::AmazonS3Builder; -use object_store::aws::AmazonS3ConfigKey; -use object_store::azure::AzureConfigKey; -use object_store::azure::MicrosoftAzureBuilder; -use object_store::gcp::GoogleCloudStorageBuilder; -use object_store::gcp::GoogleConfigKey; -use object_store::local::LocalFileSystem; +use object_store::path::Path; +use object_store::registry::ObjectStoreRegistry; use parking_lot::Mutex; use url::Url; use vortex::error::VortexError; use vortex::error::VortexResult; -use vortex::error::vortex_bail; +use vortex::error::vortex_err; use vortex::io::compat::Compat; use vortex::io::filesystem::FileSystemRef; use vortex::io::object_store::ObjectStoreFileSystem; use vortex::io::runtime::Handle; use vortex::utils::aliases::hash_map::HashMap; +use vortex_cloud::Registry; +/// Resolve `url` to a filesystem plus the path of the URL *within* it — not every scheme mounts +/// its store at the URL authority, so callers must key their reads by the returned path. pub(crate) fn object_store_fs( url: &Url, properties: &HashMap, handle: Handle, -) -> VortexResult { - let object_store = make_object_store(url, properties)?; +) -> VortexResult<(FileSystemRef, String)> { + let (object_store, path) = make_object_store(url, properties)?; let object_store = Arc::new(Compat::new(object_store)) as Arc; - Ok(Arc::new(ObjectStoreFileSystem::new(object_store, handle))) + Ok(( + Arc::new(ObjectStoreFileSystem::new(object_store, handle)), + path.to_string(), + )) } -/// Process-wide cache of constructed object stores, keyed by URL + properties so that repeated -/// requests against the same bucket/configuration share a single client. -static OBJECT_STORES: LazyLock>>> = +/// Registries keyed by the caller's properties: a store built with one caller's credentials +/// must not serve another's requests. +static REGISTRIES: LazyLock>>> = LazyLock::new(|| Mutex::new(HashMap::new())); -#[expect(clippy::cognitive_complexity)] +/// Process-wide cache of OpenDAL-backed stores, keyed by URL authority + properties. +static OPENDAL_STORES: LazyLock>>> = + LazyLock::new(|| Mutex::new(HashMap::new())); + +/// Resolve `url` to a store plus the path of the URL within it, configured from the caller's +/// `object_store` properties over the process environment. pub(crate) fn make_object_store( url: &Url, properties: &HashMap, -) -> VortexResult> { +) -> VortexResult<(Arc, Path)> { let start = std::time::Instant::now(); - // The cache key depends only on the URL authority + sorted properties, so we can hoist it - // above the scheme dispatch. This lets every store (including OpenDAL-backed ones) share - // a single client across repeated requests against the same bucket/configuration. - let cache_key = url_cache_key(url, properties); - - { - if let Some(cached) = OBJECT_STORES.lock().get(&cache_key) { - return Ok(Arc::clone(cached)); - } - // guard dropped at close of scope - } - - // OpenDAL-backed stores (Tencent COS, Alibaba OSS) use schemes that `object_store` does not - // recognize natively. Resolve them via the optional `opendal` feature, and cache the result so - // subsequent calls for the same URL share a single client. Asking `supports_scheme` rather - // than matching scheme strings here keeps this call site correct as services are added. - #[cfg(feature = "opendal")] + // OpenDAL schemes take the services' own property names (e.g. `secret_id`), not environment + // names, and mount at the URL authority, which makes the authority-keyed cache sound. if vortex_cloud::opendal::supports_scheme(url.scheme()) { + let path = Path::from_url_path(url.path()) + .map_err(|e| vortex_err!("cannot parse url path as object_store Path: {e}"))?; + let cache_key = url_cache_key(url, properties); + { + if let Some(cached) = OPENDAL_STORES.lock().get(&cache_key) { + return Ok((Arc::clone(cached), path)); + } + } let store = vortex_cloud::opendal::make_opendal_store(url, properties) .map_err(|e| VortexError::from(object_store::Error::from(e)))?; - return cache_and_return(store, url, properties, &start); + OPENDAL_STORES.lock().insert(cache_key, Arc::clone(&store)); + return Ok((store, path)); } - let (scheme, _) = ObjectStoreScheme::parse(url) - .map_err(|error| VortexError::from(object_store::Error::from(error)))?; - - // Configure extra properties on that scheme instead. - let store: Arc = match scheme { - ObjectStoreScheme::Local => { - tracing::trace!("using LocalFileSystem object store"); - Arc::new(LocalFileSystem::default()) - } - ObjectStoreScheme::AmazonS3 => { - tracing::trace!("using AmazonS3 object store"); - let mut builder = AmazonS3Builder::new() - .with_url(url.to_string()) - // Use generic S3 endpoint to avoid DNS resolution issues with region-specific endpoints - .with_endpoint("https://s3.amazonaws.com") - // Use path-style URLs - .with_virtual_hosted_style_request(false) - // Allow user to override endpoint to HTTP endpoints, e.g. LocalStack, Minio - .with_allow_http(true); - - // Try to load credentials from environment if not provided in properties - if !properties.contains_key("access_key_id") - && let Ok(access_key) = std::env::var("AWS_ACCESS_KEY_ID") - { - builder = builder.with_access_key_id(access_key); - } - if !properties.contains_key("secret_access_key") - && let Ok(secret_key) = std::env::var("AWS_SECRET_ACCESS_KEY") - { - builder = builder.with_secret_access_key(secret_key); - } - if !properties.contains_key("region") - && let Ok(region) = std::env::var("AWS_DEFAULT_REGION") - { - builder = builder.with_region(region); - } - - for (key, val) in properties { - if let Ok(config_key) = AmazonS3ConfigKey::from_str(key.as_str()) { - builder = builder.with_config(config_key, val); - } else { - tracing::warn!("Skipping unknown Amazon S3 config key: {key}"); - } - } - - Arc::new(builder.build()?) - } - ObjectStoreScheme::MicrosoftAzure => { - tracing::trace!("using MicrosoftAzure object store"); - - // NOTE(aduffy): anecdotally Azure often times out after 30 seconds, this bumps us up - // to avoid that. - let client_opts = ClientOptions::new().with_timeout(Duration::from_secs(120)); - let mut builder = MicrosoftAzureBuilder::new() - .with_url(url.to_string()) - .with_client_options(client_opts); - for (key, val) in properties { - if let Ok(config_key) = AzureConfigKey::from_str(key.as_str()) { - tracing::warn!("setting azure config {key:?} = {val}"); - builder = builder.with_config(config_key, val); - } else { - tracing::warn!("Skipping unknown Azure config key: {key}"); - } - } - - Arc::new(builder.build()?) - } - ObjectStoreScheme::GoogleCloudStorage => { - tracing::trace!("using GoogleCloudStorage object store"); - - let mut builder = GoogleCloudStorageBuilder::new().with_url(url.to_string()); - for (key, val) in properties { - if let Ok(config_key) = GoogleConfigKey::from_str(key.as_str()) { - builder = builder.with_config(config_key, val); - } else { - tracing::warn!("Skipping unknown Google Cloud Storage config key: {key}"); - } - } + let (store, path) = registry_for(properties) + .resolve(url) + .map_err(VortexError::from)?; - Arc::new(builder.build()?) - } - store => { - vortex_bail!("Unsupported store scheme: {store:?}"); - } - }; + let duration = start.elapsed(); + tracing::debug!("make_object_store latency = {duration:?}"); - cache_and_return(store, url, properties, &start) + Ok((store, path)) } -/// Insert the built store into the process-wide cache (keyed by URL + properties) and return it, -/// logging the construction latency. -fn cache_and_return( - store: Arc, - url: &Url, - properties: &HashMap, - start: &std::time::Instant, -) -> VortexResult> { - let cache_key = url_cache_key(url, properties); - OBJECT_STORES.lock().insert(cache_key, Arc::clone(&store)); - - let duration = start.elapsed(); - tracing::debug!("make_object_store latency = {duration:?}"); +/// The registry serving `properties`, created on first use. +fn registry_for(properties: &HashMap) -> Arc { + let mut sorted_props: Vec<_> = properties.iter().collect(); + sorted_props.sort_by_key(|(k, _)| *k); + let key: String = sorted_props + .iter() + .map(|(k, v)| format!("{k}={v}")) + .collect::>() + .join(","); - Ok(store) + let mut registries = REGISTRIES.lock(); + Arc::clone(registries.entry(key).or_insert_with(|| { + // Later inserts win; keys are lowercased because the registry matches case-insensitively + // and requires each key at most once. + let mut vars: HashMap = std::env::vars() + .map(|(k, v)| (k.to_ascii_lowercase(), v)) + .collect(); + vars.extend( + properties + .iter() + .map(|(k, v)| (k.to_ascii_lowercase(), v.clone())), + ); + Arc::new(Registry::with_vars(vars)) + })) } fn url_cache_key(url: &Url, properties: &HashMap) -> String { @@ -195,3 +121,81 @@ fn url_cache_key(url: &Url, properties: &HashMap) -> String { props_str, ) } + +#[cfg(test)] +mod tests { + use std::fmt::Write; + + use vortex::error::vortex_err; + + use super::*; + + fn parse(url: &str) -> VortexResult { + Url::parse(url).map_err(|e| vortex_err!("{e}")) + } + + #[test] + fn test_hf_url_reports_the_in_repository_path() -> VortexResult<()> { + let url = parse("hf://datasets/org/name/data/train.vortex")?; + let (_store, path) = make_object_store(&url, &HashMap::new())?; + + assert_eq!(path.as_ref(), "data/train.vortex"); + Ok(()) + } + + #[test] + fn test_hf_repositories_do_not_share_a_store() -> VortexResult<()> { + let a = parse("hf://datasets/org/one/train.vortex")?; + let b = parse("hf://datasets/org/two/train.vortex")?; + + let (store_a, _) = make_object_store(&a, &HashMap::new())?; + let (store_b, _) = make_object_store(&b, &HashMap::new())?; + + assert!(!Arc::ptr_eq(&store_a, &store_b)); + Ok(()) + } + + /// `object_store` offers no way to read a store's configuration back; the Debug output is + /// the only observable. + /// + /// The properties use canonical `aws_*` spellings: a property overrides the environment by + /// exact key, and CI runners export `AWS_REGION`-style variables, so a short spelling + /// (`region`) would race its environment alias at the store builder. + #[test] + #[expect(clippy::use_debug)] + fn test_s3_properties_reach_the_store() -> VortexResult<()> { + let url = parse("s3://bucket/dir/data%20file.vortex")?; + let properties = HashMap::from_iter([ + ("aws_region".to_string(), "eu-central-9".to_string()), + ( + "aws_endpoint".to_string(), + "http://localhost:9000".to_string(), + ), + ("aws_allow_http".to_string(), "true".to_string()), + ]); + + let (store, path) = make_object_store(&url, &properties)?; + assert_eq!(path.as_ref(), "dir/data file.vortex"); + + let mut debug_str = String::new(); + write!(&mut debug_str, "{store:?}").map_err(|e| vortex_err!("{e}"))?; + assert!(debug_str.contains("eu-central-9"), "{debug_str}"); + assert!(debug_str.contains("localhost:9000"), "{debug_str}"); + Ok(()) + } + + #[test] + fn test_stores_are_shared_per_property_set() -> VortexResult<()> { + let url = parse("s3://bucket/key.vortex")?; + let a = HashMap::from_iter([("aws_region".to_string(), "eu-central-9".to_string())]); + let b = HashMap::from_iter([("aws_region".to_string(), "us-west-7".to_string())]); + + let (store_a1, _) = make_object_store(&url, &a)?; + let (store_a2, _) = make_object_store(&url, &a)?; + let (store_b, _) = make_object_store(&url, &b)?; + + assert!(Arc::ptr_eq(&store_a1, &store_a2)); + assert!(!Arc::ptr_eq(&store_a1, &store_b)); + Ok(()) + } +} diff --git a/vortex-jni/src/writer.rs b/vortex-jni/src/writer.rs index 18c6a5fa101..794aa1cc65c 100644 --- a/vortex-jni/src/writer.rs +++ b/vortex-jni/src/writer.rs @@ -89,9 +89,7 @@ fn resolve_store( .map_err(|_| vortex_err!("invalid file URL: {url_or_path}"))?; Ok(ResolvedStore::Path(path)) } else { - let path = ObjectStorePath::from_url_path(url.path()) - .map_err(|_| vortex_err!("invalid object_store path: {}", url.path()))?; - let store = make_object_store(&url, properties)?; + let (store, path) = make_object_store(&url, properties)?; Ok(ResolvedStore::ObjectStore(store, path)) } }