Skip to content

Conversation

rakshil14-2
Copy link

@rakshil14-2 rakshil14-2 commented Aug 3, 2025

Issue :#4070

Description of changes:
Added a flag for deleting all versions of an object present in a versioned-enable bucket. For dealing with versions, a list-object-versions api is called to get the version ids as well as delete markers of the objects. Once the object and their version ids are retrieved, then a batch of objects with their version id is created which is passed to delete-objects api for deletion.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@rakshil14-2 rakshil14-2 marked this pull request as ready for review August 5, 2025 23:05
@aemous aemous requested a review from a team August 7, 2025 17:49
@rakshil14-2 rakshil14-2 changed the title Batch creation for delete Deleting all versions of objects Aug 7, 2025
Copy link
Contributor

@aemous aemous left a comment

Choose a reason for hiding this comment

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

Let's also generate a changelog entry.

looks good overall, a few questions/requests

Copy link
Contributor

@aemous aemous left a comment

Choose a reason for hiding this comment

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

LGTM

Rakshil Modi added 2 commits August 15, 2025 11:55
Adding response to delete handler
Updated comments

Remove BucketLister class

Added change logs

Modifying S3 Handler

Format display message

Changing names of the class

changing to marked down
@@ -89,7 +96,10 @@ def __call__(self, transfer_manager, result_queue):
command_result_recorder = CommandResultRecorder(
result_queue, result_recorder, result_processor
)

if self._requires_batch_handler():
Copy link
Member

@kdaily kdaily Aug 15, 2025

Choose a reason for hiding this comment

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

I'm not 100% sure about adding logic to change the type returned. In the function signature it says:

        :returns: A S3TransferHandler instance

Does a BatchS3TransferHandler act like the existing S3TransferHandler?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, but BatchS3TransferHandler will handler batching operations

@@ -0,0 +1,5 @@
{
"type": "enhancement",
"category": "`s3`",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think this should have two ticks on both sides. ``s3``

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants