-
Notifications
You must be signed in to change notification settings - Fork 87
feat: add preview MultipartUploadClient#listMultipartUploads #3396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
9de7d8a to
f6b31d3
Compare
| * | ||
| * @param request The request object containing the details for listing the multipart uploads. | ||
| * @return A {@link ListMultipartUploadsResponse} object containing the list of multipart uploads. | ||
| * @since 2.60.0 This new api is in preview and is subject to breaking changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update this to 2.61
…v3.54.1 (#3381) Co-authored-by: BenWhitehead <[email protected]>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: cloud-java-bot <[email protected]>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
…3395) Co-authored-by: Dhriti Chopra <[email protected]>
|
|
||
| private final String eTag; | ||
| private final String md5; | ||
| private final String crc32c; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merge main here so that these changes are removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like they are part of this PR only, please move to a seprate PR with appropriate commit message and test cases.
...ud-storage/src/main/java/com/google/cloud/storage/multipartupload/model/MultipartUpload.java
Outdated
Show resolved
Hide resolved
...rc/main/java/com/google/cloud/storage/multipartupload/model/ListMultipartUploadsRequest.java
Show resolved
Hide resolved
...c/main/java/com/google/cloud/storage/multipartupload/model/ListMultipartUploadsResponse.java
Show resolved
Hide resolved
...ud-storage/src/main/java/com/google/cloud/storage/multipartupload/model/MultipartUpload.java
Outdated
Show resolved
Hide resolved
| import java.util.stream.Collectors; | ||
|
|
||
| /** A utility class to parse {@link HttpResponse} and create a {@link UploadPartResponse}. */ | ||
| /** A utility class to parse checksums from an {@link HttpResponse}. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed for listing the in progress uploads? If not, can this fix be moved to it's own PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When moved to it's own PR, please also add tests covering the logic change in https://github.com/googleapis/java-storage/blob/main/google-cloud-storage/src/test/java/com/google/cloud/storage/ChecksumResponseParserTest.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, Those changes are moved to another PR and reverted from this PR.
94a7989 to
bda8df6
Compare
bda8df6 to
310754d
Compare
This reverts commit 21fb8fe.
...c/main/java/com/google/cloud/storage/multipartupload/model/ListMultipartUploadsResponse.java
Outdated
Show resolved
Hide resolved
| public ListMultipartUploadsResponse listMultipartUploads(ListMultipartUploadsRequest request) { | ||
| throw new UnsupportedOperationException("This operation is not yet implemented."); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are okay breaking this classes API, it's annotated @InternalExtensionOnly, and the constructor is package private preventing subclasses by anything outside our package.
To get clirr to not fail for this we should add an ignore rule to https://github.com/googleapis/java-storage/blob/main/google-cloud-storage/clirr-ignored-differences.xml
<!-- MultipartUploadClient is @InternalExtensionOnly -->
<difference>
<!-- allow new method to be added at any time -->
<differenceType>7013</differenceType>
<className>com/google/cloud/storage/MultipartUploadClient</className>
<method>* *(*)</method>
</difference>And keep this method abstract
| public ListMultipartUploadsResponse listMultipartUploads(ListMultipartUploadsRequest request) { | |
| throw new UnsupportedOperationException("This operation is not yet implemented."); | |
| } | |
| public abstract ListMultipartUploadsResponse listMultipartUploads( | |
| ListMultipartUploadsRequest request); |
| private static String formatName(String name) { | ||
| // Only lowercase letters, digits, and "-" are allowed | ||
| return name.toLowerCase().replaceAll("[^\\w\\d\\-]", "-"); | ||
| return name.toLowerCase().replaceAll("[^\\w-]", "-"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we need to remove \d from this group?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the comment wasn't updated when the regex was and is now inaccurate.
Adding model classes for ListMultipartUpload request and response.
Implementing ListMultipartUpload API.