Skip to content

Commit dc9d496

Browse files
Merge branch 'release-0.19.0'
* release-0.19.0: Bumping version to 0.19.0 Merge customizations for S3 Bump aws-actions/configure-aws-credentials in the github-actions group (#395) Bump https://github.com/astral-sh/ruff-pre-commit (#394) Bump github/codeql-action in the github-actions group (#393)
2 parents 7ae4c0e + 7a23cb3 commit dc9d496

10 files changed

Lines changed: 802 additions & 24 deletions

File tree

.changes/0.19.0.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
[
2+
{
3+
"category": "``s3``",
4+
"description": "Update multi-part copy logic to match single-part behavior for TaggingDirective and AnnotationDirective",
5+
"type": "feature"
6+
},
7+
{
8+
"category": "``s3``",
9+
"description": "Warn when Metadata or Tagging is supplied to a copy without the corresponding directive set to REPLACE, in which case the supplied value is silently ignored. This matches the CopyObject behavior.",
10+
"type": "enhancement"
11+
}
12+
]

.github/workflows/codeql.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,13 @@ jobs:
2525
persist-credentials: false
2626

2727
- name: "Run CodeQL init"
28-
uses: "github/codeql-action/init@95e58e9a2cdfd71adc6e0353d5c52f41a045d225"
28+
uses: "github/codeql-action/init@7211b7c8077ea37d8641b6271f6a365a22a5fbfa"
2929
with:
3030
config-file: "./.github/codeql.yml"
3131
languages: "python"
3232

3333
- name: "Run CodeQL autobuild"
34-
uses: "github/codeql-action/autobuild@95e58e9a2cdfd71adc6e0353d5c52f41a045d225"
34+
uses: "github/codeql-action/autobuild@7211b7c8077ea37d8641b6271f6a365a22a5fbfa"
3535

3636
- name: "Run CodeQL analyze"
37-
uses: "github/codeql-action/analyze@95e58e9a2cdfd71adc6e0353d5c52f41a045d225"
37+
uses: "github/codeql-action/analyze@7211b7c8077ea37d8641b6271f6a365a22a5fbfa"

.github/workflows/pull-request-build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ jobs:
2626
contents: read
2727
steps:
2828
- name: Configure AWS Credentials
29-
uses: aws-actions/configure-aws-credentials@ec61189d14ec14c8efccab744f656cffd0e33f37
29+
uses: aws-actions/configure-aws-credentials@e7f100cf4c008499ea8adda475de1042d6975c7b
3030
with:
3131
role-to-assume: ${{ env.IAM_ROLE_ARN }}
3232
role-session-name: PullRequestBuildGitHubAction

.pre-commit-config.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ repos:
77
- id: end-of-file-fixer
88
- id: trailing-whitespace
99
- repo: https://github.com/astral-sh/ruff-pre-commit
10-
rev: 6fec9b7edb08fd9989088709d864a7826dc74e80 # frozen: v0.15.12
10+
rev: 22f0422809455ec89ffdcf5a00170ba816e42ddb # frozen: v0.15.16
1111
hooks:
1212
- id: ruff-check
1313
args: [ --fix ]

CHANGELOG.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,13 @@
22
CHANGELOG
33
=========
44

5+
0.19.0
6+
======
7+
8+
* feature:``s3``: Update multi-part copy logic to match single-part behavior for TaggingDirective and AnnotationDirective
9+
* enhancement:``s3``: Warn when Metadata or Tagging is supplied to a copy without the corresponding directive set to REPLACE, in which case the supplied value is silently ignored. This matches the CopyObject behavior.
10+
11+
512
0.18.0
613
======
714

s3transfer/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ def __call__(self, bytes_amount):
145145
from s3transfer.exceptions import RetriesExceededError, S3UploadFailedError
146146

147147
__author__ = 'Amazon Web Services'
148-
__version__ = '0.18.0'
148+
__version__ = '0.19.0'
149149

150150

151151
logger = logging.getLogger(__name__)

s3transfer/copies.py

Lines changed: 181 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
# language governing permissions and limitations under the License.
1313
import copy
1414
import math
15+
from urllib.parse import parse_qsl
1516

1617
from botocore.exceptions import ClientError
1718

@@ -70,6 +71,8 @@ class CopySubmissionTask(SubmissionTask):
7071
'CopySourceSSECustomerKeyMD5',
7172
'MetadataDirective',
7273
'TaggingDirective',
74+
'AnnotationDirective',
75+
'Tagging',
7376
]
7477

7578
# Metadata fields to preserve for multipart copies.
@@ -91,6 +94,20 @@ class CopySubmissionTask(SubmissionTask):
9194
'ExpectedBucketOwner',
9295
]
9396

97+
GET_OBJECT_TAGGING_ARGS = ['RequestPayer', 'ExpectedBucketOwner']
98+
PUT_OBJECT_TAGGING_ARGS = [
99+
'RequestPayer',
100+
'ExpectedBucketOwner',
101+
'ChecksumAlgorithm',
102+
]
103+
LIST_OBJECT_ANNOTATIONS_ARGS = ['RequestPayer', 'ExpectedBucketOwner']
104+
GET_OBJECT_ANNOTATION_ARGS = ['RequestPayer', 'ExpectedBucketOwner']
105+
PUT_OBJECT_ANNOTATION_ARGS = [
106+
'RequestPayer',
107+
'ExpectedBucketOwner',
108+
'ChecksumAlgorithm',
109+
]
110+
94111
def _submit(
95112
self, client, config, osutil, request_executor, transfer_future
96113
):
@@ -113,6 +130,8 @@ def _submit(
113130
transfer request that tasks are being submitted for
114131
"""
115132
preserved_metadata = {}
133+
source_version_id = None
134+
call_args = transfer_future.meta.call_args
116135
if (
117136
transfer_future.meta.size is None
118137
or transfer_future.meta.etag is None
@@ -122,7 +141,6 @@ def _submit(
122141
# the TransferManager. If the object is outside of the region
123142
# of the client, they may have to provide the file size themselves
124143
# with a completely new client.
125-
call_args = transfer_future.meta.call_args
126144
head_object_request = (
127145
self._get_head_object_request_from_copy_source(
128146
call_args.copy_source
@@ -148,6 +166,9 @@ def _submit(
148166
# during a multipart copy.
149167
transfer_future.meta.provide_object_etag(response.get('ETag'))
150168
preserved_metadata = self._extract_preserved_metadata(response)
169+
# Pin the source version so all subsequent reads (tags, annotations)
170+
# are consistent with the object from the head call
171+
source_version_id = response.get('VersionId')
151172

152173
# If it is greater than threshold do a multipart copy, otherwise
153174
# do a regular copy object.
@@ -163,6 +184,7 @@ def _submit(
163184
request_executor,
164185
transfer_future,
165186
preserved_metadata,
187+
source_version_id=source_version_id,
166188
)
167189

168190
def _submit_copy_request(
@@ -199,9 +221,10 @@ def _submit_multipart_request(
199221
request_executor,
200222
transfer_future,
201223
preserved_metadata=None,
224+
source_version_id=None,
202225
):
203226
call_args = transfer_future.meta.call_args
204-
merged_extra_args = self._merge_preserved_metadata(
227+
merged_extra_args = self._apply_preserved_metadata(
205228
call_args.extra_args, preserved_metadata or {}
206229
)
207230

@@ -293,16 +316,19 @@ def _submit_multipart_request(
293316
complete_multipart_extra_args = self._extra_complete_multipart_args(
294317
call_args.extra_args
295318
)
319+
296320
# Submit the request to complete the multipart upload.
297321
self._transfer_coordinator.submit(
298322
request_executor,
299-
CompleteMultipartUploadTask(
323+
CopyCompleteMultipartUploadTask(
300324
transfer_coordinator=self._transfer_coordinator,
301325
main_kwargs={
302326
'client': client,
303327
'bucket': call_args.bucket,
304328
'key': call_args.key,
305329
'extra_args': complete_multipart_extra_args,
330+
'call_args': call_args,
331+
'source_version_id': source_version_id,
306332
},
307333
pending_main_kwargs={
308334
'upload_id': create_multipart_future,
@@ -319,15 +345,18 @@ def _extract_preserved_metadata(self, head_object_response):
319345
preserved[field] = head_object_response[field]
320346
return preserved
321347

322-
def _merge_preserved_metadata(self, extra_args, preserved_metadata):
323-
if not preserved_metadata:
324-
return extra_args
348+
def _apply_preserved_metadata(self, extra_args, preserved_metadata):
349+
# MPU has no native MetadataDirective, handle metadata manually. REPLACE
350+
# means we copy whatever the user provided, anything else means we drop
351+
# what the user supplied
325352
if extra_args.get('MetadataDirective') == 'REPLACE':
326353
return extra_args
327-
merged = dict(extra_args)
328-
for field, value in preserved_metadata.items():
329-
merged[field] = value
330-
return merged
354+
result = {
355+
k: v for k, v in extra_args.items()
356+
if k not in self.PRESERVED_METADATA_FIELDS
357+
}
358+
result.update(preserved_metadata)
359+
return result
331360

332361
def _get_head_object_request_from_copy_source(self, copy_source):
333362
if isinstance(copy_source, dict):
@@ -357,6 +386,148 @@ def _get_transfer_size(
357386
return part_size
358387

359388

389+
class CopyCompleteMultipartUploadTask(CompleteMultipartUploadTask):
390+
"""CompleteMultipartUpload variant that also applies tags and annotations.
391+
392+
After the destination object is finalized, copies/applies tags and
393+
annotations inline. Errors during apply propagate as task failures.
394+
"""
395+
396+
def _main(
397+
self,
398+
client,
399+
bucket,
400+
key,
401+
upload_id,
402+
parts,
403+
extra_args,
404+
call_args,
405+
source_version_id,
406+
):
407+
response = client.complete_multipart_upload(
408+
Bucket=bucket,
409+
Key=key,
410+
UploadId=upload_id,
411+
MultipartUpload={'Parts': parts},
412+
**extra_args,
413+
)
414+
dest_etag = response.get('ETag')
415+
dest_version_id = response.get('VersionId')
416+
self._apply_tags(client, call_args, source_version_id, dest_version_id)
417+
self._apply_annotations(
418+
client, call_args, source_version_id, dest_version_id, dest_etag
419+
)
420+
421+
def _apply_tags(self, client, call_args, source_version_id, dest_version_id):
422+
extra_args = call_args.extra_args
423+
directive = extra_args.get('TaggingDirective')
424+
if directive not in ('COPY', 'REPLACE'):
425+
return
426+
if directive == 'COPY':
427+
src_kwargs = {
428+
'Bucket': call_args.copy_source['Bucket'],
429+
'Key': call_args.copy_source['Key'],
430+
**get_filtered_dict(
431+
extra_args, CopySubmissionTask.GET_OBJECT_TAGGING_ARGS
432+
),
433+
}
434+
if source_version_id:
435+
src_kwargs['VersionId'] = source_version_id
436+
tag_set = call_args.source_client.get_object_tagging(
437+
**src_kwargs
438+
).get('TagSet', [])
439+
else: # REPLACE
440+
tag_set = [
441+
{'Key': k, 'Value': v}
442+
for k, v in parse_qsl(
443+
extra_args.get('Tagging', ''),
444+
keep_blank_values=True,
445+
)
446+
]
447+
if not tag_set:
448+
return
449+
put_kwargs = {
450+
'Bucket': call_args.bucket,
451+
'Key': call_args.key,
452+
'Tagging': {'TagSet': tag_set},
453+
**get_filtered_dict(
454+
extra_args, CopySubmissionTask.PUT_OBJECT_TAGGING_ARGS
455+
),
456+
}
457+
if dest_version_id:
458+
put_kwargs['VersionId'] = dest_version_id
459+
client.put_object_tagging(**put_kwargs)
460+
461+
def _apply_annotations(
462+
self,
463+
client,
464+
call_args,
465+
source_version_id,
466+
dest_version_id,
467+
dest_etag,
468+
):
469+
# We copy annotations only if COPY is explicitly set by the user.
470+
extra_args = call_args.extra_args
471+
if extra_args.get('AnnotationDirective') != 'COPY':
472+
return
473+
src_base = {
474+
'Bucket': call_args.copy_source['Bucket'],
475+
'Key': call_args.copy_source['Key'],
476+
}
477+
if source_version_id:
478+
src_base['VersionId'] = source_version_id
479+
list_kwargs = {
480+
**src_base,
481+
**get_filtered_dict(
482+
extra_args, CopySubmissionTask.LIST_OBJECT_ANNOTATIONS_ARGS
483+
),
484+
}
485+
get_kwargs_base = {
486+
**src_base,
487+
**get_filtered_dict(
488+
extra_args, CopySubmissionTask.GET_OBJECT_ANNOTATION_ARGS
489+
),
490+
}
491+
put_passthrough = get_filtered_dict(
492+
extra_args, CopySubmissionTask.PUT_OBJECT_ANNOTATION_ARGS
493+
)
494+
list_response = call_args.source_client.list_object_annotations(
495+
**list_kwargs
496+
)
497+
succeeded = []
498+
failed = {}
499+
for annotation in list_response.get('Annotations', []):
500+
name = annotation['AnnotationName']
501+
payload_response = call_args.source_client.get_object_annotation(
502+
**get_kwargs_base,
503+
AnnotationName=name,
504+
)
505+
put_kwargs = {
506+
'Bucket': call_args.bucket,
507+
'Key': call_args.key,
508+
'AnnotationName': name,
509+
'AnnotationPayload': payload_response['AnnotationPayload'].read(),
510+
**put_passthrough,
511+
}
512+
if dest_version_id:
513+
put_kwargs['VersionId'] = dest_version_id
514+
if dest_etag:
515+
put_kwargs['ObjectIfMatch'] = dest_etag
516+
try:
517+
client.put_object_annotation(**put_kwargs)
518+
succeeded.append(name)
519+
except Exception as e:
520+
failed[name] = e
521+
if failed:
522+
raise S3CopyFailedError(
523+
f'Failed to copy annotations to '
524+
f's3://{call_args.bucket}/{call_args.key}. '
525+
f'Succeeded: {succeeded}. '
526+
f'Failed: {list(failed.keys())}. '
527+
f'Errors: {failed}'
528+
)
529+
530+
360531
class CopyObjectTask(Task):
361532
"""Task to do a nonmultipart copy"""
362533

s3transfer/manager.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ class TransferManager:
220220
'CopySourceSSECustomerKeyMD5',
221221
'MetadataDirective',
222222
'TaggingDirective',
223+
'AnnotationDirective',
223224
]
224225

225226
ALLOWED_DELETE_ARGS = [
@@ -456,6 +457,25 @@ def copy(
456457
subscribers = []
457458
if source_client is None:
458459
source_client = self._client
460+
# Warn when Metadata/Tagging are supplied without a directive. To match
461+
# the low-level CopyObject behavior, the supplied values are silently
462+
# ignored unless the corresponding directive is set to 'REPLACE'. The
463+
# warning surfaces this so callers don't get blindsided when their
464+
# input has no effect.
465+
if extra_args.get('Metadata') and extra_args.get('MetadataDirective') is None:
466+
logger.warning(
467+
"Metadata was supplied without a metadata directive. The "
468+
"supplied metadata will be ignored and source metadata will "
469+
"be preserved. Set the metadata directive to 'REPLACE' to "
470+
"apply the supplied metadata."
471+
)
472+
if extra_args.get('Tagging') and extra_args.get('TaggingDirective') is None:
473+
logger.warning(
474+
"Tagging was supplied without a tagging directive. The "
475+
"supplied tagging will be ignored and source tags will be "
476+
"preserved. Set the tagging directive to 'REPLACE' to apply "
477+
"the supplied tagging."
478+
)
459479
self._validate_all_known_args(extra_args, self.ALLOWED_COPY_ARGS)
460480
if isinstance(copy_source, dict):
461481
self._validate_if_bucket_supported(copy_source.get('Bucket'))

0 commit comments

Comments
 (0)