From 71e0811529c0b02c79de992d675dee7e1b2edf72 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Thu, 22 May 2025 18:24:30 +0800 Subject: [PATCH 1/2] RFC: Options API Signed-off-by: Xuanwo --- core/src/docs/rfcs/0000_options_api.md | 142 +++++++++++++++++++++++++ 1 file changed, 142 insertions(+) create mode 100644 core/src/docs/rfcs/0000_options_api.md diff --git a/core/src/docs/rfcs/0000_options_api.md b/core/src/docs/rfcs/0000_options_api.md new file mode 100644 index 000000000000..6d053df28b72 --- /dev/null +++ b/core/src/docs/rfcs/0000_options_api.md @@ -0,0 +1,142 @@ +- Proposal Name: `options_api` +- Start Date: 2025-05-22 +- RFC PR: [apache/opendal#0000](https://github.com/apache/opendal/pull/0000) +- Tracking Issue: [apache/opendal#0000](https://github.com/apache/opendal/issues/0000) + +# Summary + +Introduce a new options-based API pattern for OpenDAL to simplify the construction of operations. + +# Motivation + +OpenDAL currently offers two API patterns. + +The first is a simple pattern that provides the most basic operations, such as `read`, `write`, and `delete`. These operations do not accept any options and serve as quick shortcuts for users to interact with OpenDAL. + +```rust +let data = op.read("example.txt").await?; +let _ = op.write("example.txt", data).await?; +``` + +The second is the builder pattern API, which includes methods like `read_with`, `write_with`, and `delete_with`. This approach uses a builder pattern to construct operations with customizable options, offering greater flexibility and allowing users to tailor operations to their specific needs. + +```rust +let data = op + .read_with("example.txt") + .if_match(etag).await?; +let _ = op.write_with("example.txt", data) + .chunk(1024 * 1024) + .concurrent(8) + .await?; +``` + +The builder pattern is designed to make the OpenDAL API more flexible and extensible, but it can be cumbersome for users who are trying to construct complex operations that require tuning options at runtime. + +For examples, we have seen users write code like: + +```rust +let mut write = self + .core + .write_with(&path, bs) + .append(kwargs.append.unwrap_or(false)); +if let Some(chunk) = kwargs.chunk { + write = write.chunk(chunk); +} +if let Some(content_type) = &kwargs.content_type { + write = write.content_type(content_type); +} +if let Some(content_disposition) = &kwargs.content_disposition { + write = write.content_disposition(content_disposition); +} +if let Some(cache_control) = &kwargs.cache_control { + write = write.cache_control(cache_control); +} +if let Some(user_metadata) = kwargs.user_metadata.take() { + write = write.user_metadata(user_metadata); +} +write.call().map(|_| ()).map_err(format_pyerr) +``` + +This makes the code hard to read and maintain, especially when there are many options to set. + +So I propose to introduce an option based API pattern that allows users to set options in a more structured and readable way. + +This new API will allow users to create structs like `ReadOptions` which contains all the options they want to set for an operation. They can then pass this struct to the operation, making the code cleaner and easier to understand. + +# Guide-level explanation + +We will have the following new structs: + +```rust +pub struct ReadOptions { + pub if_match: Option, + pub if_none_match: Option, +} +``` + +And we will expose APIs like: + +```rust +impl Operator { + pub async fn read_options(&self, path: &str, opts: ReadOptions) -> Result { + ... + } +} +``` + +Users can then use this API like: + +```rust +let opts = ReadOptions { + version: Some(version_id), + ...ReadOptions::default(), +}; +let data = op.read_options(path, opts).await?; +``` + +# Reference-level explanation + +We will introduce an options API by creating new structs for each operation, with each struct containing all configurable options for that specific operation. + +Before calling APIs on `oio::Access`, we will convert `ReadOptions` to `OpRead` and `OpReader`. + +# Drawbacks + +This RFC will add a new API patterns to OpenDAL, which may increase the complexity of the codebase and the learning curve for new users. + +# Rationale and alternatives + +## Expose `OpXxx` API AS-IS + +It’s a valid alternative to expose the `OpXxx` API as is. However, OpenDAL’s public API is not a direct one-to-one mapping from `Operator` to `oio::Access`. For instance, concurrency and chunk handling for read operations are implemented within the public API, which is why we have a separate `OpReader` struct. + +Therefore, we need a new struct that can handle all of these aspects simultaneously. + +# Prior art + +`object_store` has APIs like `read_opts`: + +```rust +trait ObjectStore { + async fn get_opts(&self, path: &Path, options: GetOptions) {..} +} + +pub struct GetOptions { + pub if_match: Option, + pub if_none_match: Option, + pub if_modified_since: Option>, + pub if_unmodified_since: Option>, + pub range: Option, + pub version: Option, + pub head: bool, + pub extensions: Extensions, +} +``` + +# Unresolved questions + +None + +# Future possibilities + +None From 2f92b2fb745ffd9f684963317b9dc2eec8958466 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Thu, 22 May 2025 18:42:03 +0800 Subject: [PATCH 2/2] Assign number Signed-off-by: Xuanwo --- .../docs/rfcs/{0000_options_api.md => 6213_options_api.md} | 4 ++-- core/src/docs/rfcs/mod.rs | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) rename core/src/docs/rfcs/{0000_options_api.md => 6213_options_api.md} (96%) diff --git a/core/src/docs/rfcs/0000_options_api.md b/core/src/docs/rfcs/6213_options_api.md similarity index 96% rename from core/src/docs/rfcs/0000_options_api.md rename to core/src/docs/rfcs/6213_options_api.md index 6d053df28b72..82b6676bdb4a 100644 --- a/core/src/docs/rfcs/0000_options_api.md +++ b/core/src/docs/rfcs/6213_options_api.md @@ -1,7 +1,7 @@ - Proposal Name: `options_api` - Start Date: 2025-05-22 -- RFC PR: [apache/opendal#0000](https://github.com/apache/opendal/pull/0000) -- Tracking Issue: [apache/opendal#0000](https://github.com/apache/opendal/issues/0000) +- RFC PR: [apache/opendal#6213](https://github.com/apache/opendal/pull/6213) +- Tracking Issue: [apache/opendal#6214](https://github.com/apache/opendal/issues/6214) # Summary diff --git a/core/src/docs/rfcs/mod.rs b/core/src/docs/rfcs/mod.rs index def6caa7169f..0d5fee4c03f2 100644 --- a/core/src/docs/rfcs/mod.rs +++ b/core/src/docs/rfcs/mod.rs @@ -268,3 +268,7 @@ pub mod rfc_5871_read_returns_metadata {} /// Remove Native Blocking #[doc = include_str!("6189_remove_native_blocking.md")] pub mod rfc_6189_remove_native_blocking {} + +/// Options API +#[doc = include_str!("6213_options_api.md")] +pub mod rfc_6213_options_api {}