From 64880d8d1c3a1fe6ec938d850df802edadeffc5d Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Thu, 20 Nov 2025 11:56:43 -0500 Subject: [PATCH 1/4] Improve Documentation on `ObjectStoreExt` --- src/lib.rs | 78 +++++++++++++++++------------------------------------- 1 file changed, 25 insertions(+), 53 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 31c5352..24db194 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -62,6 +62,11 @@ //! [ACID]: https://en.wikipedia.org/wiki/ACID //! [S3]: https://aws.amazon.com/s3/ //! +//! # APIs +//! +//! * [`ObjectStore`]: Core object store API +//! * [`ObjectStoreExt`]: (*New in 0.13.0*) Extension trait with additional convenience methods +//! //! # Available [`ObjectStore`] Implementations //! //! By default, this crate provides the following implementations: @@ -615,66 +620,31 @@ pub type MultipartId = String; /// Universal API to multiple object store services. /// -/// For more convenience methods, check [`ObjectStoreExt`]. +/// See the [module-level documentation](crate) for a high leve overview and +/// examples. See [`ObjectStoreExt`] for additional convenience methods. /// /// # Contract -/// This trait is meant as a contract between object store implementations -/// (e.g. providers, wrappers) and the `object_store` crate itself and is +/// This trait is a contract between object store implementations +/// (e.g. providers, wrappers) and the `object_store` crate itself. It is /// intended to be the minimum API required for an object store. /// -/// The [`ObjectStoreExt`] acts as an API/contract between `object_store` -/// and the store users and provides additional methods that may be simpler to use but overlap -/// in functionality with [`ObjectStore`]. -/// -/// # Wrappers -/// If you wrap an [`ObjectStore`] -- e.g. to add observability -- you SHOULD -/// implement all trait methods. This ensures that defaults implementations -/// that are overwritten by the wrapped store are also used by the wrapper. -/// For example: -/// -/// ```ignore -/// struct MyStore { -/// ... -/// } -/// -/// #[async_trait] -/// impl ObjectStore for MyStore { -/// // implement custom ranges handling -/// async fn get_ranges( -/// &self, -/// location: &Path, -/// ranges: &[Range], -/// ) -> Result> { -/// ... -/// } -/// -/// ... -/// } +/// The [`ObjectStoreExt`] acts as an API/contract between `object_store` and +/// the store users and provides additional methods that may be simpler to use +/// but overlap in functionality with [`ObjectStore`]. /// -/// struct Wrapper { -/// inner: Arc, -/// } +/// # No Default Implementations /// -/// #[async_trait] -/// #[deny(clippy::missing_trait_methods)] -/// impl ObjectStore for Wrapper { -/// // If we would not implement this method, -/// // we would get the trait default and not -/// // use the actual implementation of `inner`. -/// async fn get_ranges( -/// &self, -/// location: &Path, -/// ranges: &[Range], -/// ) -> Result> { -/// ... -/// } +/// There are no default implementation for any of the methods in this trait by +/// design. This was different than in versions prior to `0.13.0`, which had +/// several default implementations. Default implementations were convenient for +/// users, but was error-prone as it required implementations of ObjectStore +/// keep the convenience APIs correctly in sync. /// -/// ... -/// } -/// ``` +/// As of version 0.13.0, all methods on [`ObjectStore`] must be implemented, and +/// the convenience methods are provided by the [`ObjectStoreExt`] trait as +/// described above. See [#385] for more details. /// -/// To automatically detect this issue, use -/// [`#[deny(clippy::missing_trait_methods)]`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_trait_methods). +/// [#385]: https://github.com/apache/arrow-rs-object-store/issues/385 #[async_trait] pub trait ObjectStore: std::fmt::Display + Send + Sync + Debug + 'static { /// Save the provided `payload` to `location` with the given options @@ -1151,7 +1121,9 @@ as_ref_impl!(Box); /// Extension trait for [`ObjectStore`] with convenience functions. /// -/// See "contract" section within the [`ObjectStore`] documentation for more reasoning. +/// See the [module-level documentation](crate) for a high leve overview and +/// examples. See "contract" section within the [`ObjectStore`] documentation +/// for more reasoning. /// /// # Implementation /// You MUST NOT implement this trait yourself. It is automatically implemented for all [`ObjectStore`] implementations. From eb6bc185a64ba5cd5672479f702640f5fbd311b9 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Tue, 25 Nov 2025 08:37:36 -0500 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Kyle Barron --- src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 24db194..3291c5c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -620,16 +620,16 @@ pub type MultipartId = String; /// Universal API to multiple object store services. /// -/// See the [module-level documentation](crate) for a high leve overview and +/// See the [module-level documentation](crate) for a high level overview and /// examples. See [`ObjectStoreExt`] for additional convenience methods. /// /// # Contract -/// This trait is a contract between object store implementations +/// This trait is a contract between object store _implementations_ /// (e.g. providers, wrappers) and the `object_store` crate itself. It is /// intended to be the minimum API required for an object store. /// /// The [`ObjectStoreExt`] acts as an API/contract between `object_store` and -/// the store users and provides additional methods that may be simpler to use +/// the store _users_ and provides additional methods that may be simpler to use /// but overlap in functionality with [`ObjectStore`]. /// /// # No Default Implementations From 69eaabf0d17795c25dfa4bcc30abe909e9df89db Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Tue, 25 Nov 2025 08:40:40 -0500 Subject: [PATCH 3/4] Hedge wording for default implementaitons --- src/lib.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ff9840f..d65053c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -618,7 +618,7 @@ pub type DynObjectStore = dyn ObjectStore; /// Id type for multipart uploads. pub type MultipartId = String; -/// Universal API to multiple object store services. +/// Universal API for object store services. /// /// See the [module-level documentation](crate) for a high level overview and /// examples. See [`ObjectStoreExt`] for additional convenience methods. @@ -632,16 +632,15 @@ pub type MultipartId = String; /// the store _users_ and provides additional methods that may be simpler to use /// but overlap in functionality with [`ObjectStore`]. /// -/// # No Default Implementations +/// # Minimal Default Implementations +/// There are only a few default implementations for methods in this trait by +/// design. This was different from versions prior to `0.13.0`, which had many +/// more default implementations. Default implementations are convenient for +/// users, but error-prone for implementors as they require keeping the +/// convenience APIs correctly in sync. /// -/// There are no default implementation for any of the methods in this trait by -/// design. This was different than in versions prior to `0.13.0`, which had -/// several default implementations. Default implementations were convenient for -/// users, but was error-prone as it required implementations of ObjectStore -/// keep the convenience APIs correctly in sync. -/// -/// As of version 0.13.0, all methods on [`ObjectStore`] must be implemented, and -/// the convenience methods are provided by the [`ObjectStoreExt`] trait as +/// As of version 0.13.0, most methods on [`ObjectStore`] must be implemented, and +/// the convenience methods have been moved to the [`ObjectStoreExt`] trait as /// described above. See [#385] for more details. /// /// [#385]: https://github.com/apache/arrow-rs-object-store/issues/385 From c4c4f1c0b0dc8eb46bd1c4be5b6ceec0df3f164a Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Mon, 1 Dec 2025 15:35:23 -0500 Subject: [PATCH 4/4] revert changes to object store docs --- src/lib.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index d65053c..2fbb46b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -644,6 +644,56 @@ pub type MultipartId = String; /// described above. See [#385] for more details. /// /// [#385]: https://github.com/apache/arrow-rs-object-store/issues/385 +/// +/// # Wrappers +/// If you wrap an [`ObjectStore`] -- e.g. to add observability -- you SHOULD +/// implement all trait methods. This ensures that defaults implementations +/// that are overwritten by the wrapped store are also used by the wrapper. +/// For example: +/// +/// ```ignore +/// struct MyStore { +/// ... +/// } +/// +/// #[async_trait] +/// impl ObjectStore for MyStore { +/// // implement custom ranges handling +/// async fn get_ranges( +/// &self, +/// location: &Path, +/// ranges: &[Range], +/// ) -> Result> { +/// ... +/// } +/// +/// ... +/// } +/// +/// struct Wrapper { +/// inner: Arc, +/// } +/// +/// #[async_trait] +/// #[deny(clippy::missing_trait_methods)] +/// impl ObjectStore for Wrapper { +/// // If we would not implement this method, +/// // we would get the trait default and not +/// // use the actual implementation of `inner`. +/// async fn get_ranges( +/// &self, +/// location: &Path, +/// ranges: &[Range], +/// ) -> Result> { +/// ... +/// } +/// +/// ... +/// } +/// ``` +/// +/// To automatically detect this issue, use +/// [`#[deny(clippy::missing_trait_methods)]`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_trait_methods). #[async_trait] pub trait ObjectStore: std::fmt::Display + Send + Sync + Debug + 'static { /// Save the provided `payload` to `location` with the given options