Skip to content

Commit

Permalink
RFC-1735: Operation Extension (#1735)
Browse files Browse the repository at this point in the history
* RFC: Operation Extension

Signed-off-by: Xuanwo <github@xuanwo.io>

* Assign number

Signed-off-by: Xuanwo <github@xuanwo.io>

* Update

Signed-off-by: Xuanwo <github@xuanwo.io>

* Update core/src/docs/rfcs/1735_operation_extension.md

Co-authored-by: Suyan <suyanhanx@gmail.com>

* Update tracking issue

Signed-off-by: Xuanwo <github@xuanwo.io>

* Fix typo

Signed-off-by: Xuanwo <github@xuanwo.io>

---------

Signed-off-by: Xuanwo <github@xuanwo.io>
Co-authored-by: Suyan <suyanhanx@gmail.com>
  • Loading branch information
Xuanwo and suyanhanx committed Mar 23, 2023
1 parent 04ca6e5 commit ccd0017
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 0 deletions.
117 changes: 117 additions & 0 deletions core/src/docs/rfcs/1735_operation_extension.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
- 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#1738](https://github.com/apache/incubator-opendal/issues/1738)

# 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 used 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<String>,
+ if_match: Option<String>,
+ if_none_match: Option<String>,
+ override_content_disposition: Option<String>,
}
```

For `OpWrite`:

```diff
pub struct OpWrite {
append: bool,

content_type: Option<String>,
content_disposition: Option<String>,
+ cache_control: Option<String>,
}
```

We will provide different default options for each service. For example, we can add `default_storage_class` for `s3`.


# Drawbacks

None

# Rationale and alternatives

## 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

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.
3 changes: 3 additions & 0 deletions core/src/docs/rfcs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!("1735_operation_extension.md")]
pub mod rfc_1735_operation_extension {}

0 comments on commit ccd0017

Please sign in to comment.