Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions datafusion-cli/src/object_storage/instrumented.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ impl ObjectStoreRegistry for InstrumentedObjectStoreRegistry {
self.inner.register_store(url, store)
}

fn deregister_store(
&self,
url: &Url,
) -> datafusion::common::Result<Arc<dyn ObjectStore>> {
self.inner.deregister_store(url)
}

fn get_store(&self, url: &Url) -> datafusion::common::Result<Arc<dyn ObjectStore>> {
self.inner.get_store(url)
}
Expand Down
7 changes: 7 additions & 0 deletions datafusion/core/src/execution/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,13 @@ impl SessionContext {
self.runtime_env().register_object_store(url, object_store)
}

/// Deregisters an [`ObjectStore`] associated with the specific URL prefix.
///
/// See [`RuntimeEnv::deregister_object_store`] for more details.
pub fn deregister_object_store(&self, url: &Url) -> Result<Arc<dyn ObjectStore>> {
self.runtime_env().deregister_object_store(url)
}

/// Registers the [`RecordBatch`] as the specified table name
pub fn register_batch(
&self,
Expand Down
15 changes: 15 additions & 0 deletions datafusion/execution/src/object_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ pub trait ObjectStoreRegistry: Send + Sync + std::fmt::Debug + 'static {
store: Arc<dyn ObjectStore>,
) -> Option<Arc<dyn ObjectStore>>;

/// Deregister the store previously registered with the same key. Returns the
/// deregistered store if it existed.
fn deregister_store(&self, url: &Url) -> Result<Arc<dyn ObjectStore>>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is breaking, we can add a label. I'm thinking maybe we can have a default return value and remove it in another version but I don't know what would be a good default return value (I was thinking of just erroring).

Copy link
Member

Choose a reason for hiding this comment

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

Do you mean that giving it a default value and letting users be aware of the new API in the next release, then removing the default value in the next release after next?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but I am very open to doing it another way if there is a more standard way to deal with this.


/// Get a suitable store for the provided URL. For example:
///
/// - URL with scheme `file:///` or no scheme will return the default LocalFS store
Expand Down Expand Up @@ -230,6 +234,17 @@ impl ObjectStoreRegistry for DefaultObjectStoreRegistry {
self.object_stores.insert(s, store)
}

fn deregister_store(&self, url: &Url) -> Result<Arc<dyn ObjectStore>> {
let s = get_url_key(url);
let (_, object_store) = self.object_stores
.remove(&s)
.ok_or_else(|| {
internal_datafusion_err!("Failed to deregister object store. No suitable object store found for {url}. See `RuntimeEnv::register_object_store`")
})?;

Ok(object_store)
}

fn get_store(&self, url: &Url) -> Result<Arc<dyn ObjectStore>> {
let s = get_url_key(url);
self.object_stores
Expand Down
8 changes: 6 additions & 2 deletions datafusion/execution/src/runtime_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,6 @@ impl RuntimeEnv {
/// ```
///
/// # Example: Register remote URL object store like [Github](https://github.com)
///
///
/// ```
/// # use std::sync::Arc;
/// # use url::Url;
Expand All @@ -141,6 +139,12 @@ impl RuntimeEnv {
self.object_store_registry.register_store(url, object_store)
}

/// Deregisters a custom `ObjectStore` previously registered for a specific url.
/// See [`ObjectStoreRegistry::deregister_store`] for more details.
pub fn deregister_object_store(&self, url: &Url) -> Result<Arc<dyn ObjectStore>> {
self.object_store_registry.deregister_store(url)
}

/// Retrieves a `ObjectStore` instance for a url by consulting the
/// registry. See [`ObjectStoreRegistry::get_store`] for more
/// details.
Expand Down