Skip to content

Conversation

@aajtodd
Copy link
Contributor

@aajtodd aajtodd commented Mar 10, 2022

Speedup presign tests and bucket deletion by order of magnitude + misc cleanup

@aajtodd aajtodd requested a review from a team as a code owner March 10, 2022 17:25
@aajtodd aajtodd requested a review from ianbotsf March 10, 2022 17:25
@github-actions
Copy link

A new generated diff is ready to view: __generated-main...__generated-fix-presign-tests

Comment on lines +73 to 83
client.listObjectsV2Paginated { bucket = bucketName }
.flatMapConcat { it.contents?.asFlow() ?: flowOf() }
.collect { obj ->
val job = scope.launch(dispatcher) {
client.deleteObject {
bucket = bucketName
key = obj.key
}
}
jobs.add(job)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Not sure how many objects are being deleted but it would be more efficient to delete them in batches:

client.listObjectsV2Paginated { bucket = bucketName }
    .map { page ->
        DeleteObjectsRequest {
            bucket = bucketName
            delete {
                objects = page.contents!!.map { obj ->
                    ObjectIdentifier { key = obj.key }
                }
            }
        }
    }
    .map { req -> scope.launch { s3.deleteObjects(req) } }
    .toList()
    .joinAll()

Comment on lines -43 to +44
do {
val bucketExists = try {
client.headBucket { bucket = testBucket }
true
} catch (ex: NotFound) {
delay(300)
false
}
} while (!bucketExists)
client.waitUntilBucketExists { bucket = testBucket }
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment: Nice!

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link

A new generated diff is ready to view: __generated-main...__generated-fix-presign-tests

@aajtodd aajtodd merged commit 3517d82 into main Mar 11, 2022
@aajtodd aajtodd deleted the fix-presign-tests branch March 11, 2022 17:41
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.

2 participants