alamb commented on code in PR #348: URL: https://github.com/apache/arrow-rs-object-store/pull/348#discussion_r2084959653
########## src/registry.rs: ########## @@ -0,0 +1,243 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! ObjectStoreRegistry holds all the object stores at Runtime with a scheme for each store. +//! This allows the user to resolve a URL to an ObjectStore at runtime. Unlike +//! [`crate::parse_url`], this allows for custom logic to be executed +//! when a URL is resolved to an ObjectStore. It also serves as a cache for object stores +//! to avoid repeated creation. + +use crate::{parse_url, ObjectStore}; +use std::collections::HashMap; +use std::sync::{Arc, RwLock}; +use url::Url; + +/// [`ObjectStoreRegistry`] maps a URL to an [`ObjectStore`] instance, +/// and allows users to read from different [`ObjectStore`] +/// instances. For example the registry might be configured so that +/// +/// 1. `s3://my_bucket/lineitem/` mapped to the `/lineitem` path on an +/// AWS S3 object store bound to `my_bucket` +/// +/// 2. `s3://my_other_bucket/lineitem/` mapped to the (same) +/// `/lineitem` path on a *different* AWS S3 object store bound to +/// `my_other_bucket` +/// +/// In this particular case, the url `s3://my_bucket/lineitem/` will be provided to +/// [`ObjectStoreRegistry::get_store`] and one of three things will happen: +/// +/// - If an [`ObjectStore`] has been registered with [`ObjectStoreRegistry::register_store`] with +/// `s3://my_bucket`, that [`ObjectStore`] will be returned +/// +/// - If an AWS S3 object store can be ad-hoc discovered by the url `s3://my_bucket/lineitem/`, this +/// object store will be registered with key `s3://my_bucket` and returned. +/// +/// - Otherwise `None` will be returned, indicating that no suitable [`ObjectStore`] could +/// be found +/// +/// This allows for two different use-cases: +/// +/// 1. Systems where object store buckets are explicitly created using DDL, can register these +/// buckets using [`ObjectStoreRegistry::register_store`] +/// +/// 2. Systems relying on ad-hoc discovery, without corresponding DDL, can create [`ObjectStore`] +/// lazily by providing a custom implementation of [`ObjectStoreRegistry`] +pub trait ObjectStoreRegistry: Send + Sync + std::fmt::Debug + 'static { + /// If a store with the same key existed before, it is replaced and returned + fn register_store( + &self, + url: &Url, + store: Arc<dyn ObjectStore>, + ) -> Option<Arc<dyn ObjectStore>>; + + /// Get a suitable store for the provided URL. For example: + /// + /// - URL with scheme `file:///` or no scheme will return the default LocalFS store + /// - URL with scheme `s3://bucket/` will return the S3 store + /// - URL with scheme `hdfs://hostname:port/` will return the hdfs store + /// + /// If no [`ObjectStore`] found for the `url`, ad-hoc discovery may be executed depending on + /// the `url` and [`ObjectStoreRegistry`] implementation. An [`ObjectStore`] may be lazily + /// created and registered. + fn get_store(&self, url: &Url) -> Option<Arc<dyn ObjectStore>>; + + /// List all registered URLs + fn list_urls(&self) -> Vec<String>; +} + +/// The default [`ObjectStoreRegistry`] Review Comment: > Thinking about this more: I think we should just make DefaultObjectStoreRegistry really dumb by default. Least surprises and all. So I'm inclined to remove get_url_key from the DefaultObjectStoreRegistry and just have it return None if nothing matches a URL get_store key. I think that seems like a good idea to me > That sets up a race condition with another call that might read/modify/write for the same key. This condition could at least be detected as `register_store` returns the previously registered store -- tough it is not a great situation to have created the store twice somehow... ########## src/registry.rs: ########## @@ -0,0 +1,258 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! ObjectStoreRegistry holds all the object stores at Runtime with a scheme for each store. +//! This allows the user to resolve a URL to an ObjectStore at runtime. Unlike +//! [`crate::parse_url`], this allows for custom logic to be executed +//! when a URL is resolved to an ObjectStore. It also serves as a cache for object stores +//! to avoid repeated creation. + +use crate::{parse_url, ObjectStore}; +use std::collections::HashMap; +use std::sync::{Arc, RwLock}; +use url::Url; + +/// [`ObjectStoreRegistry`] maps a URL to an [`ObjectStore`] instance, +/// and allows users to read from different [`ObjectStore`] +/// instances. For example the registry might be configured so that +/// +/// 1. `s3://my_bucket/lineitem/` mapped to the `/lineitem` path on an +/// AWS S3 object store bound to `my_bucket` +/// +/// 2. `s3://my_other_bucket/lineitem/` mapped to the (same) +/// `/lineitem` path on a *different* AWS S3 object store bound to +/// `my_other_bucket` +/// +/// In this particular case, the url `s3://my_bucket/lineitem/` will be provided to +/// [`ObjectStoreRegistry::get_store`] and one of three things will happen: +/// +/// - If an [`ObjectStore`] has been registered with [`ObjectStoreRegistry::register_store`] with +/// `s3://my_bucket`, that [`ObjectStore`] will be returned +/// +/// - If an AWS S3 object store can be ad-hoc discovered by the url `s3://my_bucket/lineitem/`, this +/// object store will be registered with key `s3://my_bucket` and returned. +/// +/// - Otherwise `None` will be returned, indicating that no suitable [`ObjectStore`] could +/// be found +/// +/// This allows for two different use-cases: +/// +/// 1. Systems where object store buckets are explicitly created using DDL, can use the provided [`DefaultObjectStoreRegistry`] to register these +/// buckets using [`ObjectStoreRegistry::register_store`] +/// +/// 2. Systems relying on ad-hoc discovery, without corresponding DDL, can create [`ObjectStore`] +/// lazily by providing a custom implementation of [`ObjectStoreRegistry`] +pub trait ObjectStoreRegistry: Send + Sync + std::fmt::Debug + 'static { + /// Register a new store for any url's that begin with `prefix` + /// + /// If a store with the same key existed before, it is replaced and returned + fn register_store( + &self, + prefix: &Url, + store: Arc<dyn ObjectStore>, + ) -> Option<Arc<dyn ObjectStore>>; + + /// Get a suitable store for the provided URL. For example: + /// + /// - URL with scheme `file:///` or no scheme will return the default LocalFS store + /// - URL with scheme `s3://bucket/` will return the S3 store + /// - URL with scheme `hdfs://hostname:port/` will return the hdfs store + /// + /// If no [`ObjectStore`] found for the `url`, ad-hoc discovery may be executed depending on + /// the `url` and [`ObjectStoreRegistry`] implementation. An [`ObjectStore`] may be lazily + /// created and registered. + fn get_store(&self, url: &Url) -> Option<Arc<dyn ObjectStore>>; + + /// List all registered store prefixes + fn get_store_prefixes(&self) -> Vec<String>; Review Comment: Per @tustvold 's comment, it would be nice to at least document how to go from this list to a list of `Url`s Otherwise the API is not symmetric -- one registers a store via `prefix` but when listing the stores you get a URL -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
