You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "Xuanwo (via GitHub)" <gi...@apache.org> on 2024/12/18 04:54:50 UTC
[I] new feature: Add if-match support for `reader` [opendal]
Xuanwo opened a new issue, #5426:
URL: https://github.com/apache/opendal/issues/5426
### Feature Description
OpenDAL supports `read_with(path).if_match(etag)` but not `reader_with(path).if_match(etag)`, it will be great if we can support it.
### Problem and Solution
`reader` could read diffent range of file, so the etag may need to pass while perform real reading IO. The API should be carefully designed.
### Additional Context
_No response_
### Are you willing to contribute to the development of this feature?
- [ ] Yes, I am willing to contribute to the development of this feature.
--
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-unsubscribe@opendal.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] new feature: Add if-match & if-none-match support for `reader` [opendal]
Posted by "XmchxUp (via GitHub)" <gi...@apache.org>.
XmchxUp commented on issue #5426:
URL: https://github.com/apache/opendal/issues/5426#issuecomment-2564986458
Of course, you can help complete this RFC, my understanding may not be very clear.
Thank you so much @Xuanwo @meteorgan .
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] new feature: Add if-match & if-none-match support for `reader` [opendal]
Posted by "XmchxUp (via GitHub)" <gi...@apache.org>.
XmchxUp commented on issue #5426:
URL: https://github.com/apache/opendal/issues/5426#issuecomment-2560902800
Hi, I am trying to implement it.
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] new feature: Add if-match & if-none-match support for `reader` [opendal]
Posted by "XmchxUp (via GitHub)" <gi...@apache.org>.
XmchxUp commented on issue #5426:
URL: https://github.com/apache/opendal/issues/5426#issuecomment-2564948570
I can currently only think of this method `add a new api for Reader`.
```rust
pub async fn read_with_etag(
&self,
range: impl RangeBounds<u64>,
etag: Option<&str>,
if_none_match: bool,
) -> Result<Buffer> {
```
I wonder if there are any other ideas?
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] new feature: Add if-match & if-none-match support for `reader` [opendal]
Posted by "meteorgan (via GitHub)" <gi...@apache.org>.
meteorgan commented on issue #5426:
URL: https://github.com/apache/opendal/issues/5426#issuecomment-2565668704
> `Range` condition is not supported for `reader` now, should we add it ?
emmm, According to the design of `reader`, we don't need the `Range` condition for `reader_with`. Instead, we can handle the `Range` when actually reading the data.
https://github.com/apache/opendal/blob/52c96bb8e8cb4d024ccab1f415c4756447c726dd/core/src/types/read/reader.rs#L112-L115
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] new feature: Add if-match & if-none-match support for `reader` [opendal]
Posted by "meteorgan (via GitHub)" <gi...@apache.org>.
meteorgan commented on issue #5426:
URL: https://github.com/apache/opendal/issues/5426#issuecomment-2564982334
Hi, @XmchxUp . I need this feature to address #5115. so maybe we can collaborate on it. How about I propose a RFC first ? @Xuanwo
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] new feature: Add if-match & if-none-match support for `reader` [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #5426:
URL: https://github.com/apache/opendal/issues/5426#issuecomment-2564984081
> How about I propose an RFC first ? @Xuanwo
I will first listen to @XmchxUp's intentions. It would be better if he wants to propose an RFC first.
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] new feature: Add if-match & if-none-match support for `reader` [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #5426:
URL: https://github.com/apache/opendal/issues/5426#issuecomment-2563279686
> Because my understanding is that the current service backend already has this parameter, and the ETag for multiple range requests is the same value.
Hi, I'm referring to the public API of opendal: [`Operator::reader`](https://docs.rs/opendal/latest/opendal/struct.Operator.html#method.reader). When a user calls this API, they receive an `opendal::Reader`. However, at this point, the underlying requests have not yet been executed. So we don't know of the metadata yet.
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] new feature: Add if-match & if-none-match support for `reader` [opendal]
Posted by "meteorgan (via GitHub)" <gi...@apache.org>.
meteorgan commented on issue #5426:
URL: https://github.com/apache/opendal/issues/5426#issuecomment-2565018414
> Of course, you can help complete this RFC, my understanding may not be very clear.
>
I'll submit a draft RFC in the next few days, and then we can complete it together.
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] new feature: Add if-match & if-none-match support for `reader` [opendal]
Posted by "XmchxUp (via GitHub)" <gi...@apache.org>.
XmchxUp commented on issue #5426:
URL: https://github.com/apache/opendal/issues/5426#issuecomment-2563277203
@Xuanwo hi, what do you mean by real reading IO? Because my understanding is that the current service backend already has this parameter, and the ETag for multiple range requests is the same value.
```
pub fn http_get_request(
&self,
path: &str,
range: BytesRange,
args: &OpRead,
) -> Result<Request<Buffer>> {
let p = build_rooted_abs_path(&self.root, path);
let url = format!("{}{}", self.endpoint, percent_encode_path(&p));
let mut req = Request::get(&url);
if let Some(if_match) = args.if_match() {
req = req.header(IF_MATCH, if_match);
}
```
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] new feature: Add if-match & if-none-match support for `reader` [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #5426:
URL: https://github.com/apache/opendal/issues/5426#issuecomment-2564977551
Hi, @XmchxUp, I believe we need an approach to allow users to utilize different options simultaneously, including `if_match`, `if_none_match`, `if_modified_since`, and so on. So it doesn't look good to me that have a `Reader::read_with_etag`.
Maybe we can follow the same patten like `Operator::read_with` that we can have `Reader::read_with`, `Reader::into_futures_async_read_with` and `Reader::into_bytes_stream_with`.
```rust
let r = op.reader_with(path).version("version-id").await?;
let bs = r.read_with(0..1024).if_match("etag").if_modified_since(time).await?;
```
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] new feature: Add if-match & if-none-match support for `reader` [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo closed issue #5426: new feature: Add if-match & if-none-match support for `reader`
URL: https://github.com/apache/opendal/issues/5426
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] new feature: Add if-match & if-none-match support for `reader` [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #5426:
URL: https://github.com/apache/opendal/issues/5426#issuecomment-2560931528
> Hi, I am trying to implement it.
Thant's great! Since this will affect our public API, would you like to propose an RFC first?
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] new feature: Add if-match & if-none-match support for `reader` [opendal]
Posted by "meteorgan (via GitHub)" <gi...@apache.org>.
meteorgan commented on issue #5426:
URL: https://github.com/apache/opendal/issues/5426#issuecomment-2564284652
`Range` condition is not supported for `reader` now, should we add it ?
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org