From f6cd0a847076aee4bdd2b1921825c41f53f1c271 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Thu, 23 Mar 2023 19:15:32 +0800 Subject: [PATCH 1/6] RFC: Operation Extension Signed-off-by: Xuanwo --- .../src/docs/rfcs/0000_operation_extension.md | 110 ++++++++++++++++++ core/src/docs/rfcs/mod.rs | 3 + 2 files changed, 113 insertions(+) create mode 100644 core/src/docs/rfcs/0000_operation_extension.md diff --git a/core/src/docs/rfcs/0000_operation_extension.md b/core/src/docs/rfcs/0000_operation_extension.md new file mode 100644 index 00000000000..93600fae8da --- /dev/null +++ b/core/src/docs/rfcs/0000_operation_extension.md @@ -0,0 +1,110 @@ +- Proposal Name: `operation_extension` +- Start Date: 2023-03-23 +- RFC PR: [apache/incubator-opendal#0000](https://github.com/apache/incubator-opendal/pull/0000) +- Tracking Issue: [apache/incubator-opendal#0000](https://github.com/apache/incubator-opendal/issues/0000) + +# Summary + +Extend operation capabilities to support additional native features. + +# Motivation + +OpenDAL only supports a limited set of capabilities for operations. + +- `read`/`stat`: only supports `range` +- `write`: only supports `content_type` and `content_disposition` + +Our community has a strong need for more additional native features. For example: + +- [opendal#892](https://github.com/apache/incubator-opendal/issues/892) wants [Cache-Control](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control): Allow users to specify the cache control headers for the uploaded files. +- [opendal#825](https://github.com/apache/incubator-opendal/issues/825) wants [If-Match](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Match) and [If-None-Match](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match): Allow users to makes a request conditional. +- [opendal#1726](https://github.com/apache/incubator-opendal/issues/1726) wants [response-content-disposition](https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObject.html): Allow users to specify the content disposition for the downloaded files. + +All of these feature requests are essentially asking for the same thing: the capability to define supplementary arguments for operations, particularly about HTTP services. + +# Guide-level explanation + +In this RFC, we will allow users to specify the standard HTTP headers like cache_control/if_match: + +```rust +let op = OpRead::default(). + with_cache_control("max-age=3600"). + with_if_match("\"bfc13a64729c4290ef5b2c2730249c88ca92d82d\""); +let bs = o.read_with(op).await?; +``` + +Also, we will support some non-standard but widely users features like `response-content-disposition`: + +```rust +let op = OpRead::default(). + with_override_content_disposition("attachment; filename=\"foo.txt\""); +let req = op.presign_read_with("filename", Duration::hours(1), op)?; +``` + +And, finally, we will support allow specify the default headers for services. Take `s3`'s `storage_class` as an example: + +```rust +let mut builder = S3::default(); +builder.default_storage_class("STANDARD_IA"); +builder.default_cache_control("max-age=3600"); +``` + +In general, we will support the following features: + +- Allow users to specify the (non-)standard HTTP headers for operations. +- Allow users to specify the default HTTP headers for services. + +# Reference-level explanation + +We will make the following changes in OpenDAL: + +For `OpRead` & `OpStat`: + +```diff +pub struct OpRead { + br: BytesRange, ++ cache_control: Option, ++ if_match: Option, ++ if_none_match: Option, ++ override_content_disposition: Option, +} +``` + +For `OpWrite`: + +```diff +pub struct OpWrite { + append: bool, + + content_type: Option, + content_disposition: Option, ++ cache_control: Option, +} +``` + +We will provide different default options for each service. For example, we can add `default_storage_class` for `s3`. + + +# Drawbacks + +None + +# Rationale and alternatives + +None + +# Prior art + +None + +# Unresolved questions + +None + +# Future possibilities + +## Strict Mode + +Additionally, we have implemented a `strict` option for the `Operator`. If users enable this option, OpenDAL will return an error message for unsupported options. Otherwise, it will ignore them. + +For instance, if users rely on the `if_match` behavior but services like `fs` and `hdfs` do not support it natively, they can use the `op.with_strict()` function to prompt OpenDAL to return an error. diff --git a/core/src/docs/rfcs/mod.rs b/core/src/docs/rfcs/mod.rs index 9d21655ce28..00aa8509900 100644 --- a/core/src/docs/rfcs/mod.rs +++ b/core/src/docs/rfcs/mod.rs @@ -130,3 +130,6 @@ pub mod rfc_1420_object_writer {} #[doc = include_str!("1477_remove_object_concept.md")] pub mod rfc_1477_remove_object_concept {} + +#[doc = include_str!("0000_operation_extension.md")] +pub mod rfc_0000_operation_extension {} From 4dcabf679a3698f20670afbedab78803432cba0d Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Thu, 23 Mar 2023 19:17:03 +0800 Subject: [PATCH 2/6] Assign number Signed-off-by: Xuanwo --- ...000_operation_extension.md => 1735_operation_extension.md} | 2 +- core/src/docs/rfcs/mod.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename core/src/docs/rfcs/{0000_operation_extension.md => 1735_operation_extension.md} (97%) diff --git a/core/src/docs/rfcs/0000_operation_extension.md b/core/src/docs/rfcs/1735_operation_extension.md similarity index 97% rename from core/src/docs/rfcs/0000_operation_extension.md rename to core/src/docs/rfcs/1735_operation_extension.md index 93600fae8da..07926ec7d8b 100644 --- a/core/src/docs/rfcs/0000_operation_extension.md +++ b/core/src/docs/rfcs/1735_operation_extension.md @@ -1,6 +1,6 @@ - Proposal Name: `operation_extension` - Start Date: 2023-03-23 -- RFC PR: [apache/incubator-opendal#0000](https://github.com/apache/incubator-opendal/pull/0000) +- RFC PR: [apache/incubator-opendal#1735](https://github.com/apache/incubator-opendal/pull/1735) - Tracking Issue: [apache/incubator-opendal#0000](https://github.com/apache/incubator-opendal/issues/0000) # Summary diff --git a/core/src/docs/rfcs/mod.rs b/core/src/docs/rfcs/mod.rs index 00aa8509900..a0f6ec2f6c4 100644 --- a/core/src/docs/rfcs/mod.rs +++ b/core/src/docs/rfcs/mod.rs @@ -131,5 +131,5 @@ pub mod rfc_1420_object_writer {} #[doc = include_str!("1477_remove_object_concept.md")] pub mod rfc_1477_remove_object_concept {} -#[doc = include_str!("0000_operation_extension.md")] -pub mod rfc_0000_operation_extension {} +#[doc = include_str!("1735_operation_extension.md")] +pub mod rfc_1735_operation_extension {} From 32f4d1ecafb98e4b5f96fb0c5bb6a900d12171ae Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Thu, 23 Mar 2023 19:19:30 +0800 Subject: [PATCH 3/6] Update Signed-off-by: Xuanwo --- core/src/docs/rfcs/1735_operation_extension.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/core/src/docs/rfcs/1735_operation_extension.md b/core/src/docs/rfcs/1735_operation_extension.md index 07926ec7d8b..c4974dbb893 100644 --- a/core/src/docs/rfcs/1735_operation_extension.md +++ b/core/src/docs/rfcs/1735_operation_extension.md @@ -91,7 +91,14 @@ None # Rationale and alternatives -None +## Why using `override_content_disposition` instead of `response_content_disposition`? + +`response_content_disposition` is not a part of HTTP standard, it's the private API provided by `s3`. + +- `azblob` will use `x-ms-blob-content-disposition` header +- `ocios` will use `httpResponseContentDisposition` query + +OpenDAL does not accept the query as is. Instead, we have created a more readable name `override_content_disposition` to clarify its purpose. # Prior art From 2ff6f94ad6fb5e69effe5ce41c90bca42a815ff0 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Thu, 23 Mar 2023 19:44:26 +0800 Subject: [PATCH 4/6] Update core/src/docs/rfcs/1735_operation_extension.md Co-authored-by: Suyan --- core/src/docs/rfcs/1735_operation_extension.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/docs/rfcs/1735_operation_extension.md b/core/src/docs/rfcs/1735_operation_extension.md index c4974dbb893..4089a0ae409 100644 --- a/core/src/docs/rfcs/1735_operation_extension.md +++ b/core/src/docs/rfcs/1735_operation_extension.md @@ -33,7 +33,7 @@ let op = OpRead::default(). let bs = o.read_with(op).await?; ``` -Also, we will support some non-standard but widely users features like `response-content-disposition`: +Also, we will support some non-standard but widely used features like `response-content-disposition`: ```rust let op = OpRead::default(). From 76cba4a0b352ac97f011ceac9869d9e5da755a44 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Thu, 23 Mar 2023 20:34:42 +0800 Subject: [PATCH 5/6] Update tracking issue Signed-off-by: Xuanwo --- core/src/docs/rfcs/1735_operation_extension.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/docs/rfcs/1735_operation_extension.md b/core/src/docs/rfcs/1735_operation_extension.md index 4089a0ae409..5efb4347541 100644 --- a/core/src/docs/rfcs/1735_operation_extension.md +++ b/core/src/docs/rfcs/1735_operation_extension.md @@ -1,7 +1,7 @@ - Proposal Name: `operation_extension` - Start Date: 2023-03-23 - RFC PR: [apache/incubator-opendal#1735](https://github.com/apache/incubator-opendal/pull/1735) -- Tracking Issue: [apache/incubator-opendal#0000](https://github.com/apache/incubator-opendal/issues/0000) +- Tracking Issue: [apache/incubator-opendal#1738](https://github.com/apache/incubator-opendal/issues/1738) # Summary @@ -108,7 +108,7 @@ None None -# Future possibilities +# ## Strict Mode From ade885f054c9477211d58be67999b205ef6b85c4 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Thu, 23 Mar 2023 20:38:53 +0800 Subject: [PATCH 6/6] Fix typo Signed-off-by: Xuanwo --- core/src/docs/rfcs/1735_operation_extension.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/docs/rfcs/1735_operation_extension.md b/core/src/docs/rfcs/1735_operation_extension.md index 5efb4347541..ad5aad78481 100644 --- a/core/src/docs/rfcs/1735_operation_extension.md +++ b/core/src/docs/rfcs/1735_operation_extension.md @@ -108,7 +108,7 @@ None None -# +# Future possibilities ## Strict Mode