-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
HDDS-1942. Support copy during S3 multipart upload part creation #1279
Conversation
💔 -1 overall
This message was automatically generated. |
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.
@elek Thanks for working on this! The changes look good to me.
The acceptance test failures seem related. Can we also add a unit test related to same?
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
Show resolved
Hide resolved
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.
And also there are few parameters related to x-amz-copy-source, these are not implemented.
Related to eTag I think we don't have backend implemented but for x-amz-copy-source-if-unmodified-since we can use lastModified from KeyInfo and do this right?
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Thanks the review @lokeshj1703.
Sure, I created a real unit test. Will push it soon... |
Thanks the review @bharatviswa504
Yes, it's true. Let's do it in a separated jira (opened HDDS-1997). Can be better to do it in a separated phase as it's not a blocking feature for using ozone as a docker registry backend, and also it requires more code (especially more testing code) and can be easier to review in a separated step. |
1 similar comment
Thanks the review @bharatviswa504
Yes, it's true. Let's do it in a separated jira (opened HDDS-1997). Can be better to do it in a separated phase as it's not a blocking feature for using ozone as a docker registry backend, and also it requires more code (especially more testing code) and can be easier to review in a separated step. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
+1 LGTM. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
/retest |
Thanks the review @bharatviswa504 @lokeshj1703 Do you have any more concerns? Your review is still in "requested change" phase but I think I addressed the comments. |
@lokeshj1703 Do you have any more concerns? If not, can you please change the status from "requested change"? |
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.
@elek Thanks for updating the PR! The changes look good to me. +1
@elek There is an acceptance test failure in Multipart upload. Not sure if it is related here, as I see that test is passing in CI run. |
Thank you for the reviews. I have committed this to the trunk. @elek Thanks for the contribution. |
💔 -1 overall
This message was automatically generated. |
Uploads a part by copying data from an existing object as data source
Documented here:
https://docs.aws.amazon.com/AmazonS3/latest/API/mpUploadUploadPartCopy.html
See: https://issues.apache.org/jira/browse/HDDS-1942