Xuanwo commented on code in PR #5871: URL: https://github.com/apache/opendal/pull/5871#discussion_r2011875684
########## core/src/docs/rfcs/5871_read_returns_metadata.md: ########## @@ -0,0 +1,141 @@ +- Proposal Name: `read_returns_metadata` +- Start Date: 2025-03-24 +- RFC PR: [apache/opendal#5871](https://github.com/apache/opendal/pull/5871) +- Tracking Issue: [apache/opendal#5872](https://github.com/apache/opendal/issues/5872) + +# Summary + +Enhance read operations by returning metadata along with data in read operations. + +# Motivation + +Currently, read operations (`read`, `read_with`, `reader`, `reader_with`) only return the data content. Users who need metadata +during reads (like `Content-Type`, `ETag`, `version_id`, etc.) must make an additional `stat()` call. This is inefficient and +can lead to race conditions if the file is modified between the read and stat operations. + +Many storage services (like S3, GCS, Azure Blob) return metadata in their read responses. For example, S3's GetObject API returns +important metadata like `ContentType`, `ETag`, `VersionId`, `LastModified`, etc. We should expose this information to users +directly during read operations. + +# Guide-level explanation + +The read operations will be enhanced to return both data and metadata: + +```rust +// Before +let data = op.read("path/to/file").await?; +let meta = op.stat("path/to/file").await?; +if let Some(content_type) = meta.content_type() { + println!("Content-Type: {}", content_type); +} + +// After +let (data, meta) = op.read("path/to/file").await?; Review Comment: How about not to return `Metadata` for `read` call? Users who want to play with returned metadata can just call `op.reader()` and use `metadata()`. Most users use `op.read()` really don't care about it's related metadata. ########## core/src/docs/rfcs/5871_read_returns_metadata.md: ########## @@ -0,0 +1,141 @@ +- Proposal Name: `read_returns_metadata` +- Start Date: 2025-03-24 +- RFC PR: [apache/opendal#5871](https://github.com/apache/opendal/pull/5871) +- Tracking Issue: [apache/opendal#5872](https://github.com/apache/opendal/issues/5872) + +# Summary + +Enhance read operations by returning metadata along with data in read operations. + +# Motivation + +Currently, read operations (`read`, `read_with`, `reader`, `reader_with`) only return the data content. Users who need metadata +during reads (like `Content-Type`, `ETag`, `version_id`, etc.) must make an additional `stat()` call. This is inefficient and +can lead to race conditions if the file is modified between the read and stat operations. + +Many storage services (like S3, GCS, Azure Blob) return metadata in their read responses. For example, S3's GetObject API returns +important metadata like `ContentType`, `ETag`, `VersionId`, `LastModified`, etc. We should expose this information to users +directly during read operations. + +# Guide-level explanation + +The read operations will be enhanced to return both data and metadata: + +```rust +// Before +let data = op.read("path/to/file").await?; +let meta = op.stat("path/to/file").await?; +if let Some(content_type) = meta.content_type() { + println!("Content-Type: {}", content_type); +} + +// After +let (data, meta) = op.read("path/to/file").await?; +if let Some(content_type) = meta.content_type() { + println!("Content-Type: {}", content_type); +} +``` + +For reader operations, we will introduce a new method `metadata()` that returns metadata: + +```rust +// Before +let data = op.reader("path/to/file").await?.read(..).await?; +let meta = op.stat("path/to/file").await?; +if let Some(etag) = meta.etag() { + println!("ETag: {}", etag); +} + +// After +let reader = op.reader("path/to/file").await?; +let meta = reader.metada(); +if let Some(etag) = meta.etag() { + println!("ETag: {}", etag); +} +let data = reader.read(..).await?; Review Comment: Another good thing is that know we know the total size of this file. We can remove the extra `stat` call in `Reader`. ########## core/src/docs/rfcs/5871_read_returns_metadata.md: ########## @@ -0,0 +1,141 @@ +- Proposal Name: `read_returns_metadata` +- Start Date: 2025-03-24 +- RFC PR: [apache/opendal#5871](https://github.com/apache/opendal/pull/5871) +- Tracking Issue: [apache/opendal#5872](https://github.com/apache/opendal/issues/5872) + +# Summary + +Enhance read operations by returning metadata along with data in read operations. + +# Motivation + +Currently, read operations (`read`, `read_with`, `reader`, `reader_with`) only return the data content. Users who need metadata +during reads (like `Content-Type`, `ETag`, `version_id`, etc.) must make an additional `stat()` call. This is inefficient and +can lead to race conditions if the file is modified between the read and stat operations. + +Many storage services (like S3, GCS, Azure Blob) return metadata in their read responses. For example, S3's GetObject API returns +important metadata like `ContentType`, `ETag`, `VersionId`, `LastModified`, etc. We should expose this information to users +directly during read operations. + +# Guide-level explanation + +The read operations will be enhanced to return both data and metadata: + +```rust +// Before +let data = op.read("path/to/file").await?; +let meta = op.stat("path/to/file").await?; +if let Some(content_type) = meta.content_type() { + println!("Content-Type: {}", content_type); +} + +// After +let (data, meta) = op.read("path/to/file").await?; +if let Some(content_type) = meta.content_type() { + println!("Content-Type: {}", content_type); +} +``` + +For reader operations, we will introduce a new method `metadata()` that returns metadata: + +```rust +// Before +let data = op.reader("path/to/file").await?.read(..).await?; +let meta = op.stat("path/to/file").await?; +if let Some(etag) = meta.etag() { + println!("ETag: {}", etag); +} + +// After +let reader = op.reader("path/to/file").await?; +let meta = reader.metada(); +if let Some(etag) = meta.etag() { + println!("ETag: {}", etag); +} +let data = reader.read(..).await?; +``` + +The behavior remains backward compatible if users don't need the metadata. The new API will be optional, +and users can still use the existing `reader` methods without any changes. + +# Reference-level explanation + +## Changes to `Operator` API + +The following functions will be modified to return `Result<(Buffer, Metadata)>` instead of `Result<Buffer>`: + +- `read()` Review Comment: I prefer to not change `read()`. Think about this:  ########## core/src/docs/rfcs/5871_read_returns_metadata.md: ########## @@ -0,0 +1,141 @@ +- Proposal Name: `read_returns_metadata` +- Start Date: 2025-03-24 +- RFC PR: [apache/opendal#5871](https://github.com/apache/opendal/pull/5871) +- Tracking Issue: [apache/opendal#5872](https://github.com/apache/opendal/issues/5872) + +# Summary + +Enhance read operations by returning metadata along with data in read operations. + +# Motivation + +Currently, read operations (`read`, `read_with`, `reader`, `reader_with`) only return the data content. Users who need metadata +during reads (like `Content-Type`, `ETag`, `version_id`, etc.) must make an additional `stat()` call. This is inefficient and +can lead to race conditions if the file is modified between the read and stat operations. + +Many storage services (like S3, GCS, Azure Blob) return metadata in their read responses. For example, S3's GetObject API returns +important metadata like `ContentType`, `ETag`, `VersionId`, `LastModified`, etc. We should expose this information to users +directly during read operations. + +# Guide-level explanation + +The read operations will be enhanced to return both data and metadata: + +```rust +// Before +let data = op.read("path/to/file").await?; +let meta = op.stat("path/to/file").await?; +if let Some(content_type) = meta.content_type() { + println!("Content-Type: {}", content_type); +} + +// After +let (data, meta) = op.read("path/to/file").await?; +if let Some(content_type) = meta.content_type() { + println!("Content-Type: {}", content_type); +} +``` + +For reader operations, we will introduce a new method `metadata()` that returns metadata: + +```rust +// Before +let data = op.reader("path/to/file").await?.read(..).await?; +let meta = op.stat("path/to/file").await?; +if let Some(etag) = meta.etag() { + println!("ETag: {}", etag); +} + +// After +let reader = op.reader("path/to/file").await?; +let meta = reader.metada(); Review Comment: typo. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
