Xuanwo commented on code in PR #2944:
URL:
https://github.com/apache/incubator-opendal/pull/2944#discussion_r1306594924
##########
bin/oay/src/services/webdav/webdavfs.rs:
##########
@@ -156,10 +162,23 @@ impl DavFileSystem for WebdavFs {
fn rename<'a>(&'a self, from: &'a DavPath, to: &'a DavPath) ->
dav_server::fs::FsFuture<()> {
async move {
- self.op
- .rename(from.as_url_string().as_str(),
to.as_url_string().as_str())
- .await
- .map_err(convert_error)
+ let from_path = from
+ .as_rel_ospath()
+ .to_str()
+ .ok_or(FsError::GeneralFailure)?;
+ let to_path =
to.as_rel_ospath().to_str().ok_or(FsError::GeneralFailure)?;
+ let res = self.op.rename(from_path, to_path).await;
+ match res {
+ Ok(_) => Ok(()),
+ Err(e) => {
+ if from.is_collection() {
Review Comment:
Please don't handling error like this.
- If the services doesn't support move a dir, opendal should return
`Unsupported`.
- If the services raise error for the file exists, opendal should return
`AlreadyExists`
- If the services raise error for trying to rename a folder to file, opendal
should return `NotADirectory`
Otherwise, it's a bug that we need to fix.
Maybe we can check `to` first before calling `rename`?
##########
bin/oay/src/services/webdav/webdavfs.rs:
##########
@@ -156,10 +162,23 @@ impl DavFileSystem for WebdavFs {
fn rename<'a>(&'a self, from: &'a DavPath, to: &'a DavPath) ->
dav_server::fs::FsFuture<()> {
async move {
- self.op
- .rename(from.as_url_string().as_str(),
to.as_url_string().as_str())
- .await
- .map_err(convert_error)
+ let from_path = from
+ .as_rel_ospath()
+ .to_str()
+ .ok_or(FsError::GeneralFailure)?;
+ let to_path =
to.as_rel_ospath().to_str().ok_or(FsError::GeneralFailure)?;
+ let res = self.op.rename(from_path, to_path).await;
+ match res {
+ Ok(_) => Ok(()),
+ Err(e) => {
+ if from.is_collection() {
Review Comment:
Please don't handling error like this.
- If the services doesn't support move a dir, opendal should return
`Unsupported`.
- If the services raise error for the file exists, opendal should return
`AlreadyExists`
- If the services raise error for trying to rename a folder to file, opendal
should return `NotADirectory`
Otherwise, it's a bug that we need to fix.
---
Maybe we can check `to` first before calling `rename`?
--
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]
