You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "Lzzzzzt (via GitHub)" <gi...@apache.org> on 2024/07/08 09:56:12 UTC
[PR] Feat(core): Add object versioning for OSS [opendal]
Lzzzzzt opened a new pull request, #4870:
URL: https://github.com/apache/opendal/pull/4870
This pull request add support of object versioning for OSS
Part of #2611
Basically, done the following task
- [x] change the args of `oss_head_object` and `oss_head_object_request` to receive the version info
- [x] add `versionId` param of the url sent (get, head, del) to the oss server
- [x] change the reader in `types/read/reader` to let reader send head req with the same version info of get req
--
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: [PR] feat(core): Add object versioning for OSS [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4870:
URL: https://github.com/apache/opendal/pull/4870#discussion_r1668391978
##########
core/src/raw/http_util/header.rs:
##########
@@ -186,6 +196,10 @@ pub fn parse_into_metadata(path: &str, headers: &HeaderMap) -> Result<Metadata>
m.set_content_disposition(v);
}
+ if let Some(v) = parse_version_id(headers)? {
Review Comment:
Hi, please implement those logic in oss instead of `raw`.
##########
core/src/services/oss/core.rs:
##########
@@ -47,6 +47,8 @@ mod constants {
pub const X_OSS_SERVER_SIDE_ENCRYPTION_KEY_ID: &str = "x-oss-server-side-encryption-key-id";
pub const RESPONSE_CONTENT_DISPOSITION: &str = "response-content-disposition";
+
+ pub const OBJECT_VERSIONING: &str = "versionId";
Review Comment:
How about using name `OSS_QUERY_VERSION_ID`?
##########
core/src/services/oss/backend.rs:
##########
@@ -440,10 +440,7 @@ impl Access for OssBackend {
}
async fn stat(&self, path: &str, args: OpStat) -> Result<RpStat> {
- let resp = self
- .core
- .oss_head_object(path, args.if_match(), args.if_none_match())
- .await?;
+ let resp = self.core.oss_head_object(path, &args).await?;
Review Comment:
Thanks a lot for this change.
##########
core/src/types/read/reader.rs:
##########
@@ -133,7 +133,7 @@ impl Reader {
let size = self
.ctx
.accessor()
- .stat(self.ctx.path(), OpStat::new())
+ .stat(self.ctx.path(), get_stat_op_from_read_op(self.ctx.args()))
Review Comment:
At current stage, only version is meaningful. How about just extract version? We can remove `get_stat_op_from_read_op`.
--
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: [PR] feat(core): Add object versioning for OSS [opendal]
Posted by "Lzzzzzt (via GitHub)" <gi...@apache.org>.
Lzzzzzt commented on code in PR #4870:
URL: https://github.com/apache/opendal/pull/4870#discussion_r1668400930
##########
core/src/raw/http_util/header.rs:
##########
@@ -186,6 +196,10 @@ pub fn parse_into_metadata(path: &str, headers: &HeaderMap) -> Result<Metadata>
m.set_content_disposition(v);
}
+ if let Some(v) = parse_version_id(headers)? {
Review Comment:
👌
--
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: [PR] feat(core): Add object versioning for OSS [opendal]
Posted by "Lzzzzzt (via GitHub)" <gi...@apache.org>.
Lzzzzzt commented on code in PR #4870:
URL: https://github.com/apache/opendal/pull/4870#discussion_r1668420463
##########
core/src/types/read/reader.rs:
##########
@@ -133,7 +133,7 @@ impl Reader {
let size = self
.ctx
.accessor()
- .stat(self.ctx.path(), OpStat::new())
+ .stat(self.ctx.path(), get_stat_op_from_read_op(self.ctx.args()))
Review Comment:
> > i think both of the `OpStat` and `OpRead` should be the same.
>
> The reader might send chunked read request, so they might be different.
ok
--
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: [PR] feat(core): Add object versioning for OSS [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4870:
URL: https://github.com/apache/opendal/pull/4870#discussion_r1668417479
##########
core/src/types/read/reader.rs:
##########
@@ -133,7 +133,7 @@ impl Reader {
let size = self
.ctx
.accessor()
- .stat(self.ctx.path(), OpStat::new())
+ .stat(self.ctx.path(), get_stat_op_from_read_op(self.ctx.args()))
Review Comment:
> i think both of the `OpStat` and `OpRead` should be the same.
The reader might send chunked read request, so they might be different.
--
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: [PR] feat(core): Add object versioning for OSS [opendal]
Posted by "Lzzzzzt (via GitHub)" <gi...@apache.org>.
Lzzzzzt commented on code in PR #4870:
URL: https://github.com/apache/opendal/pull/4870#discussion_r1668403411
##########
core/src/types/read/reader.rs:
##########
@@ -133,7 +133,7 @@ impl Reader {
let size = self
.ctx
.accessor()
- .stat(self.ctx.path(), OpStat::new())
+ .stat(self.ctx.path(), get_stat_op_from_read_op(self.ctx.args()))
Review Comment:
ok, but my opinion is because the reader will call this to get content length, i think both of the `OpStat` and `OpRead` should be the same.
--
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: [PR] feat(core): Add object versioning for OSS [opendal]
Posted by "Lzzzzzt (via GitHub)" <gi...@apache.org>.
Lzzzzzt commented on code in PR #4870:
URL: https://github.com/apache/opendal/pull/4870#discussion_r1668403633
##########
core/src/services/oss/core.rs:
##########
@@ -47,6 +47,8 @@ mod constants {
pub const X_OSS_SERVER_SIDE_ENCRYPTION_KEY_ID: &str = "x-oss-server-side-encryption-key-id";
pub const RESPONSE_CONTENT_DISPOSITION: &str = "response-content-disposition";
+
+ pub const OBJECT_VERSIONING: &str = "versionId";
Review Comment:
👌
--
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: [PR] feat(core): Add object versioning for OSS [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #4870:
URL: https://github.com/apache/opendal/pull/4870
--
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