You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "Wenbin1002 (via GitHub)" <gi...@apache.org> on 2025/01/01 09:55:40 UTC
[PR] feat(core): Add is_current to metadata [opendal]
Wenbin1002 opened a new pull request, #5493:
URL: https://github.com/apache/opendal/pull/5493
# Which issue does this PR close?
Closes #5474
# Rationale for this change
`Metadata` now contains `version` for the given file, but there is no way for users to different if this version the latest version of file.
# What changes are included in this PR?
add `is_current` field to metadata and initialize it in s3 and oss service
# Are there any user-facing changes?
no
--
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 is_current to metadata [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #5493:
URL: https://github.com/apache/opendal/pull/5493#discussion_r1900516454
##########
core/src/services/oss/backend.rs:
##########
@@ -521,8 +521,11 @@ impl Access for OssBackend {
let headers = resp.headers();
let mut meta = self.core.parse_metadata(path, resp.headers())?;
+ // If version id exists, set the version; otherwise, mark as the current version.
if let Some(v) = parse_header_to_str(headers, "x-oss-version-id")? {
meta.set_version(v);
+ } else {
+ meta.set_is_current(true);
Review Comment:
Hi, this check is wrong. `x-oss-version-id` doesn't mean it's an old version. Let's just not set `is_current` while performing `stat`.
##########
core/src/services/s3/backend.rs:
##########
@@ -1016,8 +1016,11 @@ impl Access for S3Backend {
meta.with_user_metadata(user_meta);
}
+ // If version id exists, set the version; otherwise, mark as the current version.
Review Comment:
The same to `oss`.
##########
core/src/types/metadata.rs:
##########
@@ -404,6 +406,27 @@ impl Metadata {
self
}
+ /// Is_current of this entry.
+ ///
+ /// Is_current is a boolean that can be used to identify
+ /// if the version of this entry is the latest version.
+ pub fn is_current(&self) -> Option<bool> {
+ self.is_current
+ }
+
+ /// Set is_current of this entry.
+ ///
+ /// For HeadObject without version_id, we will set it to Some(true).
+ ///
+ /// For HeadObject with version_id, we will set it to None.
+ ///
+ /// For ListObjects, we will set all keys to Some(true)
+ ///
+ /// For ListObjectVersions, we will decide the value based on IsLatest.
Review Comment:
```suggestion
/// Set the `is_current` status of this entry.
///
/// By default, this value will be `None`. Please avoid using this API if it's unclear whether the entry is current.
/// Set it to `true` if it is known to be the latest; otherwise, set it to `false`.
```
##########
core/src/types/metadata.rs:
##########
@@ -404,6 +406,27 @@ impl Metadata {
self
}
+ /// Is_current of this entry.
+ ///
+ /// Is_current is a boolean that can be used to identify
+ /// if the version of this entry is the latest version.
Review Comment:
```suggestion
/// Determines if the provided metadata reflects the current status of the path.
///
/// - `Ok(true)` indicates it is the latest status.
/// - `Ok(false)` indicates it is an older version of the file.
/// - `None` indicates uncertainty about its status.
///
/// This API allows users to verify if the version is up-to-date when listing with versions.
```
##########
core/src/types/metadata.rs:
##########
@@ -404,6 +406,27 @@ impl Metadata {
self
}
+ /// Is_current of this entry.
+ ///
+ /// Is_current is a boolean that can be used to identify
+ /// if the version of this entry is the latest version.
+ pub fn is_current(&self) -> Option<bool> {
+ self.is_current
+ }
+
+ /// Set is_current of this entry.
+ ///
+ /// For HeadObject without version_id, we will set it to Some(true).
+ ///
+ /// For HeadObject with version_id, we will set it to None.
+ ///
+ /// For ListObjects, we will set all keys to Some(true)
+ ///
+ /// For ListObjectVersions, we will decide the value based on IsLatest.
+ pub fn set_is_current(&mut self, is_current: bool) {
+ self.is_current = Some(is_current);
+ }
+
Review Comment:
Please add an API `with_is_current(&mut self, is_current: Option<bool>)`.
--
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 is_current to metadata [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #5493:
URL: https://github.com/apache/opendal/pull/5493
--
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 is_current to metadata [opendal]
Posted by "Wenbin1002 (via GitHub)" <gi...@apache.org>.
Wenbin1002 commented on PR #5493:
URL: https://github.com/apache/opendal/pull/5493#issuecomment-2567369514
I’ve updated the code based on your comments. PTAL, thanks!
--
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