-
Notifications
You must be signed in to change notification settings - Fork 494
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-10645. Support x-amz-metadata-directive in CopyObject #6490
Conversation
@kerneltime @DaveTeng0 @SaketaChalamchala @vtutrinov Could you help take a look when you have time? |
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.
Thanks @ivandika3 for the patch.
} else { | ||
OS3Exception ex = newError(INVALID_ARGUMENT, metadataCopyDirective); | ||
ex.setErrorMessage("An error occurred (InvalidArgument) " + | ||
"when calling the CopyObjet operation: " + |
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 calling the CopyObjet operation: " + | |
"when calling the CopyObject operation: " + |
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.
Thank you for catching it. Updated.
OS3Exception ex = newError(INVALID_ARGUMENT, metadataCopyDirective); | ||
ex.setErrorMessage("An error occurred (InvalidArgument) " + | ||
"when calling the CopyObjet operation: " + | ||
"The metadata directive specified is invalid. The valid values are COPY or REPLACE."); |
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.
"The metadata directive specified is invalid. The valid values are COPY or REPLACE."); | |
"The metadata directive specified is invalid. Valid values are COPY or REPLACE."); |
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.
Updated.
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.
Thanks @ivandika3 for updating the patch, LGTM. I'll wait some time for another review.
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.
Thank you @ivandika3 for the patch, the changes look good.
Thanks @ivandika3 for the patch, @tanvipenumudy for the review. |
Thank you for the reviews @adoroszlai @tanvipenumudy |
(cherry picked from commit e81f5c0)
(cherry picked from commit e81f5c0)
(cherry picked from commit e81f5c0)
(cherry picked from commit e81f5c0)
What changes were proposed in this pull request?
Need to support x-amz-metadata-directive: specifies whether the metadata is copied from the source object or replaced with metadata that's provided in the request. When copying an object, you can preserve all metadata (the default) or specify new metadata. If this header isn’t specified, COPY is the default behavior.
Valid Values: COPY | REPLACE
See: https://docs.aws.amazon.com/AmazonS3/latest/API/API_CopyObject.html#API_CopyObject_RequestSyntax
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10645
How was this patch tested?
Unit test and acceptance test.
Clean CI run: https://github.com/ivandika3/ozone/actions/runs/8589734492