erickguan commented on code in PR #7637:
URL: https://github.com/apache/opendal/pull/7637#discussion_r3467362501
##########
core/services/webdav/src/config.rs:
##########
@@ -75,6 +75,19 @@ pub struct WebdavConfig {
///
/// Default: `https://opendal.apache.org/ns`
pub user_metadata_uri: Option<String>,
+ /// Disable conditional read headers on GET requests.
+ ///
+ /// By default, OpenDAL advertises and sends the RFC 7232 headers
+ /// `If-Match`, `If-None-Match`, `If-Modified-Since` and
+ /// `If-Unmodified-Since` when callers ask for conditional reads.
+ ///
+ /// Some WebDAV-compatible servers (e.g., nginx-dav) don't return
+ /// ETags in PROPFIND or don't honor these conditions on GET.
+ /// Enable this option to drop the four `read_with_if_*` capabilities
+ /// so callers fail fast instead of silently losing the condition.
+ ///
+ /// Default: false
Review Comment:
```suggestion
/// Disable conditional read.
///
/// When enable conditional read, which is the default,
/// OpenDAL advertises and sends the RFC 7232 headers when provided
arguments when reading data:
/// - `If-Match`
/// - `If-None-Match`
/// - `If-Modified-Since`
/// - `If-Unmodified-Since`
///
/// When set to `true`, attempt using conditional read options causes a
`ErrorKind::Unsupported` exception.
///
/// Some WebDAV-compatible servers (e.g., nginx-dav) don't return
/// ETags in PROPFIND or don't honor these headers on GET.
///
/// Default: false
```
I also recommend you to change this to positive tone.
##########
core/services/webdav/src/backend.rs:
##########
@@ -153,6 +153,23 @@ impl WebdavBuilder {
}
self
}
+
+ /// Disable conditional read headers on GET requests.
+ ///
+ /// By default, OpenDAL advertises and sends the RFC 7232 headers
+ /// `If-Match`, `If-None-Match`, `If-Modified-Since` and
+ /// `If-Unmodified-Since` when callers ask for conditional reads.
+ ///
+ /// Some WebDAV-compatible servers (e.g., nginx-dav) don't return
+ /// ETags in PROPFIND or don't honor these conditions on GET.
+ /// Enable this option to drop the four `read_with_if_*` capabilities
+ /// so callers fail fast instead of silently losing the condition.
Review Comment:
Does this follow your intend?
```suggestion
/// Disable conditional read.
///
/// When enable conditional read, which is the default,
/// OpenDAL advertises and sends the RFC 7232 headers when provided
arguments when reading data:
/// - `If-Match`
/// - `If-None-Match`
/// - `If-Modified-Since`
/// - `If-Unmodified-Since`
///
/// Some WebDAV-compatible servers (e.g., nginx-dav) don't return
/// ETags in PROPFIND or don't honor these headers on GET.
///
/// Set this option to `true` to disable conditional read, specifically,
disabling capabilities:
/// - `read_with_if_match`
/// - `read_with_if_none_match`
/// - `read_with_if_modified_since`
/// - `read_with_if_unmodified_since`
///
/// Users would get `ErrorKind::Unsupported` when trying to read with
conditions.
```
Also, I would go with `enable_conditional_read` with default true to avoid
double negate in documentation. You ponder when reading documentation with
double negate. It generally hints more intricate context.
##########
core/services/webdav/src/core.rs:
##########
@@ -1537,6 +1559,9 @@ mod error {
StatusCode::NOT_FOUND => (ErrorKind::NotFound, false),
// Some services (like owncloud) return 403 while file locked.
StatusCode::FORBIDDEN => (ErrorKind::PermissionDenied, true),
+ StatusCode::PRECONDITION_FAILED | StatusCode::NOT_MODIFIED => {
Review Comment:
Good!
```suggestion
// Some WebDAV-compatible servers (e.g., nginx-dav) don't return
ETags in PROPFIND
StatusCode::PRECONDITION_FAILED | StatusCode::NOT_MODIFIED => {
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
