erickguan commented on code in PR #5226:
URL: https://github.com/apache/opendal/pull/5226#discussion_r1850176496


##########
core/src/layers/complete.rs:
##########
@@ -177,8 +177,19 @@ impl<A: Access> CompleteAccessor<A> {
             return Ok(RpStat::new(Metadata::new(EntryMode::DIR)));
         }
 
-        // Forward to inner if create_dir is supported.
-        if path.ends_with('/') && capability.create_dir {
+        if path.ends_with('/')

Review Comment:
   Here is the problem. The Google Drive service supports `create_dir,` so 
OpenDAL will remove the metadata without changes.
   
   ```rust
           // Forward to inner if create_dir is supported.
           if path.ends_with('/') && capability.create_dir {
               let meta = self.inner.stat(path, args).await?.into_metadata();
   
               if meta.is_file() {
                   return Err(Error::new(
                       ErrorKind::NotFound,
                       "stat expected a directory, but found a file",
                   ));
               }
   
               return Ok(RpStat::new(Metadata::new(EntryMode::DIR)));
           }
   ```
   
   I read the RFC 3243, and I am skeptical of the change in this PR. I am not 
familiar with the context so I have a few ideas in mind:
   1. Add a comment explaining the hack to clean up later.
   2. Maybe extend the complete layer to respond `meta` without creating a new 
one.



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

Reply via email to