Xuanwo commented on code in PR #5144:
URL: https://github.com/apache/opendal/pull/5144#discussion_r1778943394
##########
core/src/types/operator/operator.rs:
##########
@@ -256,6 +256,25 @@ impl Operator {
/// # }
/// ```
///
+ /// ## `version`
+ ///
+ /// Set `version` for this `stat` request.
+ ///
+ /// This feature can be used to retrieve the file metadata that matches a
specified version.
Review Comment:
Hi, the way about discussing `version` is a bit confusing to me. Unlike
`ETag`, `Version` is not used for matching. Instead, we request the specified
version directly. We will return `NotFound` if the requested version does not
exist for the given path.
##########
core/src/types/operator/operator.rs:
##########
@@ -650,6 +689,26 @@ impl Operator {
/// # }
/// ```
///
+ /// ## `version`
+ ///
+ /// Set `version` for this `reader`.
+ ///
+ /// By default, OpenDAL reads a file using the current version. By setting
the `version`, OpenDAL will read
+ /// the file with the specified version.
+ ///
+ /// If the file exists, but the version doesn't match, an error with kind
[`ErrorKind::NotFound`]
Review Comment:
the same.
##########
core/src/types/operator/operator.rs:
##########
@@ -1840,6 +1941,24 @@ impl Operator {
/// # }
/// ```
///
+ /// ## `version`
+ ///
+ /// Specify whether to list all object versions or not
+ ///
+ /// if `version` is not set, or is set to false, only the active files
with the current version will be returned.
+ ///
+ /// when `version` is set to true and the backend service has `versioning`
enabled, all object versions will be returned.
+ /// if `versioning` is not enabled, an error with kind
[`ErrorKind::Unsupported`] will be returned.
Review Comment:
I will discuss this at https://github.com/apache/opendal/pull/5145.
Generally, I don't want to declare it as part of our public API (or expected
behavior).
##########
core/src/types/operator/operator.rs:
##########
@@ -1636,6 +1719,24 @@ impl Operator {
/// # }
/// ```
///
+ /// ## `version`
+ ///
+ /// Specify whether to list all object versions or not
Review Comment:
The behavior we implemented is "list files along with their versions."
Please always remember that OpenDAL's vision is "accessing data freely." We
should do our best to distinguish OpenDAL's API from the underlying storage
services' implementation details.
##########
core/src/types/operator/operator.rs:
##########
@@ -1279,6 +1338,30 @@ impl Operator {
///
/// - Deleting a file that does not exist won't return errors.
///
+ /// # Options
+ ///
+ /// ## `version`
+ ///
+ /// Set `version` for this `delete` request.
+ ///
+ /// If `versioning` is not enabled, a `delete` request will permanently
remove the file.
+ ///
+ /// when `versioning` is enabled, a simple `delete` request won't
permanently remove the file,
+ /// it can still be accessed using its version.
+ /// By setting the `version`, OpenDAL will permanently remove the file
with the specified version.
Review Comment:
Hi, it's better not to discuss "permanently delete" here. This behavior is
determined by the service itself.
##########
core/src/types/operator/operator.rs:
##########
@@ -531,6 +550,26 @@ impl Operator {
/// # }
/// ```
///
+ /// ## `version`
+ ///
+ /// Set `version` for this `read` request.
+ ///
+ /// By default, OpenDAL reads a file using the current version. By setting
the `version`, OpenDAL will read
+ /// the file with the specified version.
+ ///
+ /// If the file exists, but the version doesn't match, an error with kind
[`ErrorKind::NotFound`]
Review Comment:
the same.
##########
core/src/types/operator/operator.rs:
##########
@@ -1840,6 +1941,24 @@ impl Operator {
/// # }
/// ```
///
+ /// ## `version`
+ ///
+ /// Specify whether to list all object versions or not
+ ///
+ /// if `version` is not set, or is set to false, only the active files
with the current version will be returned.
+ ///
+ /// when `version` is set to true and the backend service has `versioning`
enabled, all object versions will be returned.
Review Comment:
As previously discussed, please refrain from disclosing details about
backend services.
##########
core/src/types/operator/operator.rs:
##########
@@ -1840,6 +1941,24 @@ impl Operator {
/// # }
/// ```
///
+ /// ## `version`
+ ///
+ /// Specify whether to list all object versions or not
+ ///
+ /// if `version` is not set, or is set to false, only the active files
with the current version will be returned.
Review Comment:
> if `version` is not set, or is set to false,
If `version` is not enabled?
##########
core/src/types/operator/operator.rs:
##########
@@ -256,6 +256,25 @@ impl Operator {
/// # }
/// ```
///
+ /// ## `version`
+ ///
+ /// Set `version` for this `stat` request.
+ ///
+ /// This feature can be used to retrieve the file metadata that matches a
specified version.
+ ///
+ /// If file exists, but the version doesn't match, an error with kind
[`ErrorKind::NotFound`]
Review Comment:
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-unsubscr...@opendal.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
