You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "meteorgan (via GitHub)" <gi...@apache.org> on 2025/01/03 15:38:28 UTC

[I] bug: the implement of `list_with_deleted` is not consistent with the RFC-5495 [opendal]

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

   ### Describe the bug
   
   In [RFC-5495](https://github.com/apache/opendal/blob/main/core/src/docs/rfcs/5495_list_with_deleted.md):
   https://github.com/apache/opendal/blob/c866d4bbae50f863b22ec2728011e80b633177a8/core/src/docs/rfcs/5495_list_with_deleted.md?plain=1#L35
   
   But in the implement of `S3`, `list_with_deleted` only contains `delete marker` and `common prefix`, current files are not included.
   https://github.com/apache/opendal/blob/c866d4bbae50f863b22ec2728011e80b633177a8/core/src/services/s3/lister.rs#L225-L250d
   
   ### Steps to Reproduce
   
   ```rust
   pub async fn test_list_files_with_deleted(op: Operator) -> Result<()> {
       if !op.info().full_capability().list_with_deleted {
           return Ok(());
       }
   
       let parent = TEST_FIXTURE.new_dir_path();
       let file_name = TEST_FIXTURE.new_file_path();
       let file_path = format!("{}{}", parent, file_name);
       op.write(file_path.as_str(), "1").await?;
       op.write(file_path.as_str(), "2").await?;
       op.delete(file_path.as_str()).await?;
   
       let file_path2 = format!("{}{}", parent, TEST_FIXTURE.new_file_path());
       op.write(file_path2.as_str(), "1").await?;
   
       let mut ds = op.list_with(&parent).deleted(true).await?;
       // this will fail !
       assert_eq!(ds.len(), 2);
   }
   ```
   
   ### Expected Behavior
   
   the result `list_with_deleted` includes the current files or change the RFC
   
   ### Additional Context
   
   _No response_
   
   ### Are you willing to submit a PR to fix this bug?
   
   - [ ] Yes, I would like to submit a PR.


-- 
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] bug: the implement of `list_with_deleted` is not consistent with the RFC-5495 [opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #5507:
URL: https://github.com/apache/opendal/issues/5507#issuecomment-2569466215

   Yes, we should include the current file since `list_with().deleted(true)` is defined to be inclusive.


-- 
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] bug: the implement of `list_with_deleted` is not consistent with the RFC-5495 [opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo closed issue #5507: bug: the implement of `list_with_deleted` is not consistent with the RFC-5495
URL: https://github.com/apache/opendal/issues/5507


-- 
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] bug: the implement of `list_with_deleted` is not consistent with the RFC-5495 [opendal]

Posted by "hoslo (via GitHub)" <gi...@apache.org>.
hoslo commented on issue #5507:
URL: https://github.com/apache/opendal/issues/5507#issuecomment-2574866152

   > Yes, we should include the current file since `list_with().deleted(true)` is defined to be inclusive.
   
   If deleted file not latest, current file is should in output.version. And should list_with().deleted(true) include multi delete marker for same file?


-- 
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] bug: the implement of `list_with_deleted` is not consistent with the RFC-5495 [opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #5507:
URL: https://github.com/apache/opendal/issues/5507#issuecomment-2574965669

   > If the deleted file is not the latest, current file is should in output.version.
   
   @meteorgan's case mixed up two files, the most simple one should be:
   
   ```rust
   pub async fn test_list_files_with_deleted(op: Operator) -> Result<()> {
       if !op.info().full_capability().list_with_deleted {
           return Ok(());
       }
   
       let parent = TEST_FIXTURE.new_dir_path();
       let file_name = TEST_FIXTURE.new_file_path();
       let file_path = format!("{}{}", parent, file_name);
       op.write(file_path.as_str(), "1").await?;
   
       let mut ds = op.list_with(&file_path).deleted(true).await?;
       assert_eq!(ds.len(), 1);
   }
   ```
   
   This means `list_with().deleted(true)` should always include the latest version of the file.
   
   > And should list_with().deleted(true) include multi delete marker for same file?
   
   Yes, we should list all of them.
   


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