Xuanwo commented on code in PR #2779: URL: https://github.com/apache/incubator-opendal/pull/2779#discussion_r1284959009
########## core/src/docs/rfcs/2779_list_with_metakey.md: ########## @@ -0,0 +1,126 @@ +- Proposal Name: `list_with_metakey` +- Start Date: 2023-08-04 +- RFC PR: [apache/incubator-opendal#2779](https://github.com/apache/incubator-opendal/pull/2779) +- Tracking Issue: [apache/incubator-opendal#0000](https://github.com/apache/incubator-opendal/issues/0000) + +# Summary + +Move `Operator` `metadata` API to `list_with().metakey()` to simplify the usage. + +# Motivation + +The current `Entry` metadata API is: + +```rust +use opendal::Entry; +use opendal::Metakey; + +let meta = op + .metadata(&entry, Metakey::ContentLength | Metakey::ContentType) + .await?; +``` + +This API is difficult to understand and rarely used correctly. And in reality, users always fetch the same set of metadata during listing. + +Take one of our users code as an example: + +```rust +let stream = self + .inner + .scan(&path) + .await + .map_err(|err| format_object_store_error(err, &path))?; + +let stream = stream.then(|res| async { + let entry = res.map_err(|err| format_object_store_error(err, ""))?; + let meta = self + .inner + .metadata(&entry, Metakey::ContentLength | Metakey::LastModified) + .await + .map_err(|err| format_object_store_error(err, entry.path()))?; + + Ok(format_object_meta(entry.path(), &meta)) +}); + +Ok(stream.boxed()) +``` + +By moving metadata to `lister`, our user code can be simplified to: + +```rust +let stream = self + .inner + .scan_with(&path) + .metakey(Metakey::ContentLength | Metakey::LastModified) + .await + .map_err(|err| format_object_store_error(err, &path))?; + +let stream = stream.then(|res| async { + let entry = res.map_err(|err| format_object_store_error(err, ""))?; + let meta = entry.into_metadata() + + Ok(format_object_meta(entry.path(), &meta)) +}); + +Ok(stream.boxed()) +``` + +By introducing this change: + +- Users don't need to capture `Operator` in the closure. +- Users don't need to do async call like `metadata()` again. + +If we don't have this change: + +- every place that could receive a `fn()` must use `Fn()` instead which enforce users to have a generic parameter in their code. +- It's harder for other languages binding to implement `op.metadata()` right. + +# Guide-level explanation + +The new API will be: + +```rust +let entries: Vec<Entry> = op + .list_with("dir") + .metakey(Metakey::ContentLength | Metakey::ContentType).await?; + +let meta: &Metadata = entries[0].metadata(); +``` + +Metadata can be queried directly when listing entries via `metadata()`, and later extracted via `into_parts()`. + +# Reference-level explanation + +We will add `metakey` into `OpList`. Underlying services can use those information to try their best to fetch the metadata. + +There are following possibilities: + +- The entry metadata is met: `Lister` return the entry directly +- The entry metadata is not met and not fully filled: `Lister` will try to send `stat` call to fetch the metadata +- The entry metadata is not met and fully filled: `Lister` will return the entry directly. Review Comment: Metakey is more like a request query. It's something like: Hi, I expect this entry to have `content_type` and `etag`. However, it is possible that the service does not have this metadata at all. For instance, if `fs` lacks `etag`, the service will assign `None` to `etag` to imply that the entry does not possess `etag`. This is the final outcome, and there is no need to call `stat` again. -- 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: commits-unsubscr...@opendal.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
