Skip to content

Comments

feat(core): Implement list with deleted for s3 service#5498

Merged
Xuanwo merged 4 commits intomainfrom
implement-list-with-deleted
Jan 2, 2025
Merged

feat(core): Implement list with deleted for s3 service#5498
Xuanwo merged 4 commits intomainfrom
implement-list-with-deleted

Conversation

@Xuanwo
Copy link
Member

@Xuanwo Xuanwo commented Jan 2, 2025

Which issue does this PR close?

Part of #5496

Rationale for this change

Implement the RFC we need.

What changes are included in this PR?

  • Add deleted in OpList
  • Add is_deleted in Metadata
  • Implement tests for list with deleted
  • Implement list with deleted for s3 services

Are there any user-facing changes?

Users can now list deleted files from s3 services.

@github-actions github-actions bot added the releases-note/feat The PR implements a new feature or has a title that begins with "feat" label Jan 2, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Jan 2, 2025

CodSpeed Performance Report

Merging #5498 will improve performances by 18.88%

Comparing implement-list-with-deleted (a739a68) with main (7a83013)

Summary

⚡ 4 improvements
✅ 69 untouched benchmarks

Benchmarks breakdown

Benchmark main implement-list-with-deleted Change
256 KiB * 1000k chunk 183.6 ns 154.4 ns +18.88%
256 KiB * 100k chunk 183.6 ns 154.4 ns +18.88%
256 KiB * 10k chunk 183.6 ns 154.4 ns +18.88%
256 KiB * 1k chunk 212.8 ns 183.6 ns +15.89%

Signed-off-by: Xuanwo <[email protected]>
Signed-off-by: Xuanwo <[email protected]>
@Xuanwo Xuanwo requested review from dqhl76, hoslo and meteorgan January 2, 2025 13:12
Copy link
Contributor

@hoslo hoslo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Xuanwo Xuanwo merged commit 0146a12 into main Jan 2, 2025
277 checks passed
@Xuanwo Xuanwo deleted the implement-list-with-deleted branch January 2, 2025 13:27
if let Some(etag) = version_object.etag {
meta.set_etag(&etag);
meta.set_content_md5(etag.trim_matches('"'));
if self.args.deleted() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did I misunderstand something ? this implementation seems to differ from the RFC description:

Please note that `deleted` here means "including deleted files" rather than "only deleted files." Therefore, `list_with(path).deleted(true)` will list both current files and deleted ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

releases-note/feat The PR implements a new feature or has a title that begins with "feat"

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants