[Hackathon] Add method to S3Client: deleteBucketAndAllContents #3070
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cenedhryn
reviewed
Mar 2, 2022
| } | ||
|
|
||
| s3.deleteBucket(DeleteBucketRequest.builder().bucket(bucketName).build()); | ||
| s3.deleteBucketAndAllContents(bucketName); |
There was a problem hiding this comment.
Red code is the best code!
| createBucket(BUCKET); | ||
| s3.putBucketVersioning(r -> r | ||
| .bucket(BUCKET) | ||
| .versioningConfiguration(v -> v.status(BucketVersioningStatus.ENABLED))); |
There was a problem hiding this comment.
Should we have a test that sets up a more complicated bucket where objects have different versions?
There was a problem hiding this comment.
That's a good point. I could rewrite the same first ~50 key names. Would that be sufficient?
| */ | ||
| @SdkExtensionMethod | ||
| default void deleteBucketAndAllContents(String bucket) { | ||
| new DeleteBucketAndAllContents((S3Client) this).deleteBucketAndAllContents(bucket); |
There was a problem hiding this comment.
Delegation has again proven to be more flexible. Good choice using a separate class.
| import software.amazon.awssdk.utils.Validate; | ||
|
|
||
| @SdkInternalApi | ||
| public class DeleteBucketAndAllContents implements S3ClientSdkExtension { |
There was a problem hiding this comment.
In the spirit of a hackathon I'm biasing more towards the integ tests for coverage right now. Would like to defer unit tests for now just since they're time consuming, but will absolutely require them before a potential public release.
| * S3Client#listObjectVersions(ListObjectVersionsRequest)} APIs to list a bucket's content, buffer keys that are eligible for | ||
| * deletion into batches of 1000, and delete them in bulk with the {@link S3Client#deleteObjects(DeleteObjectsRequest)} API. | ||
| * <p> | ||
| * While this method is optimized to use batch APIs for both listing and deleting, it may not be suitable for buckets |
There was a problem hiding this comment.
It may also be useful to direct people to TransferManager for their more complicated needs.
There was a problem hiding this comment.
Good point, but I don't think we currently plan to support "delete directory" in TM, but it makes sense to support it there.
| private void deleteObjectStream(String bucket, Stream<ObjectIdentifier> stream) { | ||
| Iterator<List<ObjectIdentifier>> batchIterator = batch(stream.iterator(), MAX_DELETE_OBJECTS_SIZE); | ||
| batchIterator.forEachRemaining(objects -> { | ||
| log.debug(() -> String.format("Deleting %s objects: %s", objects.size(), objects)); |
There was a problem hiding this comment.
What does this line look like in the log, specifically objects?
There was a problem hiding this comment.
It logs the key names, which may not be appropriate. I was considering removing the logs altogether for that reason.
Here's an example from CopySourceIntegrationTest:
Deleting S3 bucket: copy-source-integ-test-src-lyncbenn-2036
10:18:10.213 [main] DEBUG software.amazon.awssdk.services.s3.internal.extensions.DeleteBucketAndAllContents - Emptying bucket: copy-source-integ-test-src-lyncbenn-2036
10:18:10.398 [main] DEBUG software.amazon.awssdk.services.s3.internal.extensions.DeleteBucketAndAllContents - Deleting 5 objects: [ObjectIdentifier(Key=%20), ObjectIdentifier(Key=key/with/slashes), ObjectIdentifier(Key=simpleKey), ObjectIdentifier(Key=specialChars/ +!#$&'()*,:;=?@"), ObjectIdentifier(Key=🪣)]
10:18:10.943 [main] DEBUG software.amazon.awssdk.services.s3.internal.extensions.DeleteBucketAndAllContents - Deleting bucket: copy-source-integ-test-src-lyncbenn-2036
Deleting S3 bucket: copy-source-integ-test-versioned-src-lyncbenn-1687
10:18:11.235 [main] DEBUG software.amazon.awssdk.services.s3.internal.extensions.DeleteBucketAndAllContents - Emptying bucket: copy-source-integ-test-versioned-src-lyncbenn-1687
10:18:11.335 [main] DEBUG software.amazon.awssdk.services.s3.internal.extensions.DeleteBucketAndAllContents - Deleting 5 objects: [ObjectIdentifier(Key=%20), ObjectIdentifier(Key=key/with/slashes), ObjectIdentifier(Key=simpleKey), ObjectIdentifier(Key=specialChars/ +!#$&'()*,:;=?@"), ObjectIdentifier(Key=🪣)]
10:18:11.909 [main] DEBUG software.amazon.awssdk.services.s3.internal.extensions.DeleteBucketAndAllContents - Deleting 20 objects: [ObjectIdentifier(Key=%20, VersionId=V.sVzY.75kBdHEqJM39by_Z5sTBHZ7fQ), ObjectIdentifier(Key=%20, VersionId=sWggSroHE_WHRIOSZaDKZ2brqRu7yZhO), ObjectIdentifier(Key=%20, VersionId=fhPc80IYqCW9CjP.STRJNF2B1vlj4wHB), ObjectIdentifier(Key=key/with/slashes, VersionId=Gav4gvn8A5Ip.Ofmu8w0qmDyZWeFOI_T), ObjectIdentifier(Key=key/with/slashes, VersionId=7AhHansYt7WA5ON1ZEJ0EE0QREBE4.Pk), ObjectIdentifier(Key=key/with/slashes, VersionId=NsKZD8LYIBXs_YECP54aNSJ0QDgFgbYm), ObjectIdentifier(Key=simpleKey, VersionId=H5nEdAB_uTfl7ZA05WG0875hlfpoRn.y), ObjectIdentifier(Key=simpleKey, VersionId=doUGPN_k40B.sh8mGSGooGFn4TKvW1XE), ObjectIdentifier(Key=simpleKey, VersionId=3isxbpv5bC4u0SWKP_32YZ_h7d7VW_Y4), ObjectIdentifier(Key=specialChars/ +!#$&'()*,:;=?@", VersionId=dOYfzJl.xJe.ipWuuilM1M3jtLSYU7Dz), ObjectIdentifier(Key=specialChars/ +!#$&'()*,:;=?@", VersionId=duQuW7pjfUxRim5080eXlAZ83C8CfbRk), ObjectIdentifier(Key=specialChars/ +!#$&'()*,:;=?@", VersionId=VjBGda03yhKoVHCkz6c6DyiqXk3XAZH3), ObjectIdentifier(Key=🪣, VersionId=2IDkpsK3YOGXjvPefVkEOVk0aj7_clYf), ObjectIdentifier(Key=🪣, VersionId=IA2lPaG1Kf57vtCv2ebcVA_h12aHVdLC), ObjectIdentifier(Key=🪣, VersionId=RcMPfGgo0ejdLy_mlDXiHP5w1B9nsR_d), ObjectIdentifier(Key=%20, VersionId=Q25OFLzsmtzUIiU7cIDEZ2AWBH1IgQSJ), ObjectIdentifier(Key=key/with/slashes, VersionId=4bUxcv.SPPW0WwkejxKU9X2PnAz1XkRO), ObjectIdentifier(Key=simpleKey, VersionId=kUHpkEd3zuL6k2kkXfSe5QYTHlGPaKnE), ObjectIdentifier(Key=specialChars/ +!#$&'()*,:;=?@", VersionId=xof9bJbMxbR7WhXsQ8wGFHfn.1rWuioL), ObjectIdentifier(Key=🪣, VersionId=ZfRQ8UITh3_qflMbXHOCV9nRKP9AKcMK)]
10:18:12.300 [main] DEBUG software.amazon.awssdk.services.s3.internal.extensions.DeleteBucketAndAllContents - Deleting bucket: copy-source-integ-test-versioned-src-lyncbenn-1687
Deleting S3 bucket: copy-source-integ-test-dest-lyncbenn-7871
10:18:12.830 [main] DEBUG software.amazon.awssdk.services.s3.internal.extensions.DeleteBucketAndAllContents - Emptying bucket: copy-source-integ-test-dest-lyncbenn-7871
10:18:12.915 [main] DEBUG software.amazon.awssdk.services.s3.internal.extensions.DeleteBucketAndAllContents - Deleting 5 objects: [ObjectIdentifier(Key=%20), ObjectIdentifier(Key=key/with/slashes), ObjectIdentifier(Key=simpleKey), ObjectIdentifier(Key=specialChars/ +!#$&'()*,:;=?@"), ObjectIdentifier(Key=🪣)]
10:18:13.653 [main] DEBUG software.amazon.awssdk.services.s3.internal.extensions.DeleteBucketAndAllContents - Deleting bucket: copy-source-integ-test-dest-lyncbenn-7871
There was a problem hiding this comment.
Ok, agreed to remove user info.
cenedhryn
approved these changes
Mar 2, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.