[I] Tracking issue of RFC-7818: rename with if-not-exist [opendal]

["erickguan (via GitHub)" <gi...@apache.org>]

erickguan opened a new issue, #7828:
URL: https://github.com/apache/opendal/issues/7828

   RFC PR: #7818 
   
   ---
   
   Implement new rename API for services:
   
   - [ ] HDFS
   - [ ] webdav
   - [ ] ...
   
   Since rename API doesn't guarantee atomicity. #7825 tracks atomicity problem. 


-- 
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] Tracking issue of RFC-7818: rename with if-not-exist [opendal]

["PDGGK (via GitHub)" <gi...@apache.org>]

PDGGK commented on issue #7828:
URL: https://github.com/apache/opendal/issues/7828#issuecomment-5301760557

   Two more boxes, both verified against a running server rather than by reading:
   
   - **webdav** — #8089. RFC 4918 `Overwrite: F`; the existing `412` mapping already gives `ConditionNotMatch`.
   - **hdfs_native** — #8090. Needs the check before the destination is deleted, plus a mapping for the pre-create that can lose a race.
   
   One note on the list itself: **HDFS is already done** — `services/hdfs` sets `rename_with_if_not_exists: true` and handles it in `hdfs_rename`, so that box can be ticked as-is.
   
   `azfile` and `azdls` are the obvious next ones, but their CI fixtures load credentials through `1Password/load-secrets-action`, so a fork PR gets no behaviour coverage on them. I would rather not advertise the capability on a code reading alone — happy to write them if someone who can run that CI wants to take the verification.
   


-- 
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] Tracking issue of RFC-7818: rename with if-not-exist [opendal]

["PDGGK (via GitHub)" <gi...@apache.org>]

PDGGK commented on issue #7828:
URL: https://github.com/apache/opendal/issues/7828#issuecomment-5301454043

   I'd like to take the **webdav** box, and would rather state the shape here first than guess it in a PR.
   
   The mechanism is the one the RFC already names: `webdav_move` currently hardcodes `Overwrite: T` (`core/services/webdav/src/core.rs:380`), so the change is to thread `OpRename` through `backend.rs:385` and send `F` when `if_not_exists` is set. The error side needs nothing — `PRECONDITION_FAILED` is already mapped to `ErrorKind::ConditionNotMatch` at `core.rs:1647`, which is what RFC 4918 §9.9.4 says a server returns when `Overwrite: F` finds the destination occupied.
   
   **The one thing I'd like a steer on: how to expose the capability.** There are two recent precedents pointing opposite ways.
   
   * #7562 deprecated the webdav config toggles in favour of reporting native capabilities and using `CapabilityOverrideLayer` for tests.
   * #7637 then added `enable_conditional_read`, a config flag.
   
   My inclination is a plain `rename_with_if_not_exists: true`, no flag, and the reason is that `Overwrite` is not an optional extension: RFC 4918 §10.6 makes it part of MOVE, and a server that ignores it is non-conforming. That is unlike conditional *read*, where real servers genuinely differ on `If-Match` support and a flag earns its keep. But if you would rather have it behind a flag for consistency with #7637, say so and I will do that instead.
   
   Two smaller things I noticed while reading, neither blocking:
   
   1. `webdav_move` calls `webdav_mkcol(get_parent(to))` before issuing the MOVE. Under `if_not_exists` a 412 can therefore leave a freshly created empty parent behind. The destination itself is untouched, so I don't think it violates the RFC, but I can skip the mkcol when the flag is set if you'd prefer.
   2. The behaviour tests at `core/tests/behavior/async_rename.rs:220/251/287` key off the capability bit, so they activate on their own — no new tests needed for the happy path. There are 7 fixtures under `.github/services/webdav`, all credential-free, so this gets real coverage.
   
   If nobody objects in the next while I'll open a PR against webdav only, and can follow with `hdfs_native` afterwards (also docker-verifiable). I'd leave `azfile`/`azdls` alone — their CI needs 1Password secrets, so a fork PR would get no behaviour coverage there and I'd be asking you to enable an atomicity-gated capability on a code reading alone.
   


-- 
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