Use default size for ListObjectsRequest and ListObjectsRequestV2#456
Use default size for ListObjectsRequest and ListObjectsRequestV2#456larsj-blip wants to merge 5 commits intoBIBSYSDEV:mainfrom
Conversation
…null pointer exception
|
forgot to build first :P |
There was a problem hiding this comment.
Generally, it's polite to include a project's owner in the PR — we haven't restricted who can approve things, but it doesn't mean we want to accept all and any changes :) [Edit: I see this is a draft]
The naming for the PR, probably "Use default size for ListObjectsRequest and ListObjectsRequestV2"
| private static List<String> FetchAllExpectedFilesWithOnlyRequiredArguments(FakeS3Client s3Client, String bucket) { | ||
| var listingRequestWithOnlyTheRequiredArguments = ListObjectsRequest.builder().bucket(bucket).build(); | ||
| var listedFiles = s3Client.listObjects(listingRequestWithOnlyTheRequiredArguments); | ||
| return extractListedKeys(listedFiles); | ||
| } | ||
| private static List<String> FetchAllExpectedFilesWithOnlyRequiredArgumentsUsingV2Request(FakeS3Client s3Client, | ||
| String bucket) { | ||
| var listingRequestWithOnlyTheRequiredArguments = ListObjectsV2Request.builder().bucket(bucket).build(); | ||
| var listedFiles = s3Client.listObjectsV2(listingRequestWithOnlyTheRequiredArguments); | ||
| return extractListedKeysForV2Request(listedFiles); | ||
| } |
There was a problem hiding this comment.
Method names should begin with lowercase letter.
| } | ||
|
|
||
| @Test | ||
| void shouldAcceptListObjectsRequestWithOnlyRequiredArguments() { |
There was a problem hiding this comment.
Naming is a bit weird…possibly shouldReturnDefaultSizeListObjectResponseWhenRequestDoesNotSpecifySize, which seems to be the point.
| } | ||
|
|
||
| @Test | ||
| void shouldAcceptListObjectsV2RequestWithOnlyRequiredArguments(){ |
There was a problem hiding this comment.
Similarly, but with V2 in there…
| var listedFiles = s3Client.listObjects(listingRequestWithOnlyTheRequiredArguments); | ||
| return extractListedKeys(listedFiles); | ||
| } | ||
| private static List<String> FetchAllExpectedFilesWithOnlyRequiredArgumentsUsingV2Request(FakeS3Client s3Client, |
There was a problem hiding this comment.
A lack of space indicates that Orestis didn't write this (joke, but fix the formatting in the file).
| insertRandomFileToS3(s3Client, bucket).toString()); | ||
| } | ||
|
|
||
| private static List<String> FetchAllExpectedFilesWithOnlyRequiredArguments(FakeS3Client s3Client, String bucket) { |
There was a problem hiding this comment.
I'd name this: fetchDefaultSizeFilesListUsingBucketNameOnlyV1
| var listedFiles = s3Client.listObjects(listingRequestWithOnlyTheRequiredArguments); | ||
| return extractListedKeys(listedFiles); | ||
| } | ||
| private static List<String> FetchAllExpectedFilesWithOnlyRequiredArgumentsUsingV2Request(FakeS3Client s3Client, |
There was a problem hiding this comment.
I'd name this: fetchDefaultSizeFilesListUsingBucketNameOnlyV2
|
You'll want to pull main before the next review too :) |
…nly-required-list-objects-args
larsj-blip
changed the title
|
Hi, sorry about not adding a reviewer! Not very experienced in contributing to open source projects. Got the formatter working now :P |
No stress :) If you want a review, you'll need to re-request a review :) |
2 participants
only supplying the bucket now results in an unhandled null pointer exception