Adding No overwrite for copy operation#9592
Merged
aemous merged 2 commits intoJul 25, 2025
Merged
Conversation
aemous
requested changes
Jul 17, 2025
Contributor
aemous
left a comment
aemous
left a comment
There was a problem hiding this comment.
Looks good overall, very nice test coverage!
I requested some changes, most of them are minor. I had one concern with how we are handling copies of 0 byte objects with no-overwrite.
rakshil14-2
force-pushed
the
no_overwrite_feature_for_copy_operation
branch
3 times, most recently
from
71f54ae to
ee0b977
Compare
aemous
requested changes
Jul 21, 2025
Contributor
aemous
left a comment
aemous
left a comment
There was a problem hiding this comment.
Looks good overall, a couple minor requests, and we'll discuss potentially using LIstObjectsV2 instead of HeadObject offline.
rakshil14-2
force-pushed
the
no_overwrite_feature_for_copy_operation
branch
from
89dcfbd to
79e2504
Compare
kdaily
requested changes
Jul 22, 2025
rakshil14-2
force-pushed
the
no_overwrite_feature_for_copy_operation
branch
from
f39bcd6 to
039f4b6
Compare
rakshil14-2
commented
Jul 24, 2025
kdaily
requested changes
Jul 24, 2025
Member
kdaily
left a comment
kdaily
left a comment
There was a problem hiding this comment.
Good start! I think the logic makes sense. I think that the function names and documentation could be clarified a bit.
Also, see the suggestion about where the check is determined. There is an existing list of warning handlers that is used to skip files - could the logic be handled there, since it only needs the FileInfo object to determine?
Also, I noticed that there is a "flicker" in the terminal output when skipping a file with this. We might need to check how the progress status is updated in this case.
kdaily
reviewed
Jul 24, 2025
kdaily
approved these changes
Jul 25, 2025
updated linting changes Handling Zero byte edge case Fixed Exception type Updated doc string Updated docs string Fixed doc string Update transfer manager for copy operation
improved test case and format Updated test cases Fixed edge case for zero bytes Update warning handler
rakshil14-2
force-pushed
the
no_overwrite_feature_for_copy_operation
branch
from
211e9e4 to
75678ed
Compare
aemous
approved these changes
Jul 25, 2025
Contributor
aemous
left a comment
aemous
left a comment
There was a problem hiding this comment.
LGTM. I made one non-blocking formatting-related comment.
rakshil14-2
added a commit
to rakshil14-2/aws-cli
that referenced
this pull request
Jul 25, 2025
This reverts commit 2493f8b.
kdaily
pushed a commit
that referenced
this pull request
Jul 29, 2025
kdaily
pushed a commit
that referenced
this pull request
Jul 29, 2025
kdaily
pushed a commit
that referenced
this pull request
Aug 11, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue : Github Issue #2874
Description of changes:
Customers requested to prevent overwriting of objects. This is done by providing
no-overwriteheader usingifNoneMatchwith Etags to transfer objects that are not present on the bucket. However, retrieval of etags is difficult hence during copy operation always perform multipart copy since it follows multipart upload approach which indeed supportsifNoneMatchheader without Etags. Theno-overwriteheader works well with othercpcommand headers. Customer can be implemented usingaws s3 cp <source> <destination> --no-overwritewhich allows successful transfers when object with the same name is not present on the destination. However, if the user tries to transfer the object which is already present at the target bucket using command line, awarningabout skipping the file will be generated and that object is not allowed to transfer to S3 bucket.Testing:
Functional testing is performed to validate the object copy process. This included both scenarios copying an object when object with same key is already present in the bucket as well as those when the object with same key is not present on bucket. These tests were conducted for both object having size less than multipart threshold and one with greater than multipart threshold