Skip to content

Per-file request parameters are not applied consistently to multipart upload requests #946

Description

@laughingman7743

Problem

Per-file request parameters in s3_additional_kwargs are applied inconsistently to the requests of a multipart upload:

  1. Duplicate RequestPayer. S3FileSystem._call() passes **kwargs, **self.request_kwargs (pyathena/filesystem/s3.py:2120). With requester_pays=True on the filesystem and RequestPayer in s3_additional_kwargs, every request that forwards s3_additional_kwargs raises TypeError: retry_api_call() got multiple values for keyword argument 'RequestPayer' (for example, CreateMultipartUpload, PutObject, and GetObject).
  2. Not forwarded to part requests. S3File passes s3_additional_kwargs to CreateMultipartUpload and PutObject. It does not pass them to UploadPart, UploadPartCopy, CompleteMultipartUpload, or the abort in _finish_multipart_upload() (s3.py:2043, :2073, :2098, :1342). So:
    • A per-file RequestPayer (cross-account requester-pays bucket without the filesystem-level requester_pays=True) creates the upload, but the part uploads and the cleanup abort lack the payer acknowledgement.
    • SSE-C parameters (SSECustomerAlgorithm, SSECustomerKey, SSECustomerKeyMD5) are sent with CreateMultipartUpload but not with UploadPart. According to the S3 API reference, each part upload must repeat them.
    • ExpectedBucketOwner is checked only on some of the requests.

Expected: the request-level parameters (RequestPayer, ExpectedBucketOwner, SSE-C) apply to every request of the upload, and a parameter given in both places does not raise TypeError. Object-level parameters (metadata, ContentType, StorageClass, …) should still go only to the requests that accept them, as #929 did for discard().

Reproduction

Checked offline on master f8e6b86:

from types import SimpleNamespace
from unittest import mock
from pyathena.filesystem.s3 import S3File, S3FileSystem

# 1. duplicate RequestPayer
fs = S3FileSystem.__new__(S3FileSystem)
fs._client = mock.MagicMock()
fs._retry_config = mock.MagicMock()
fs.request_kwargs = {"RequestPayer": "requester"}
try:
    fs._call("create_multipart_upload", Bucket="b", Key="k", RequestPayer="requester")
except TypeError as e:
    print(e)  # pyathena.util.retry_api_call() got multiple values for keyword argument 'RequestPayer'

# 2. part requests without the per-file parameters
fs = mock.MagicMock(spec=S3FileSystem)
fs.MULTIPART_UPLOAD_MIN_PART_SIZE = 4
fs.MULTIPART_UPLOAD_MAX_PART_SIZE = 64
fs.exists.return_value = False
fs._create_multipart_upload.return_value = SimpleNamespace(upload_id="u")
fs._upload_part.side_effect = lambda **kw: SimpleNamespace(etag="e", part_number=kw["part_number"])
kw = {"RequestPayer": "requester", "SSECustomerAlgorithm": "AES256", "SSECustomerKey": "k"}
with S3File(fs, "s3://bucket/key", mode="wb", block_size=4, s3_additional_kwargs=kw) as f:
    f.write(b"x" * 8)
print(fs._create_multipart_upload.call_args.kwargs)  # includes RequestPayer and SSE-C
print(fs._upload_part.call_args.kwargs)              # bucket, key, upload_id, part_number, body only

The failure against real S3 (an authorization error for requester pays, or a missing-SSE-C-key error) was not reproduced; it follows from the S3 API reference.

Environment

  • PyAthena master f8e6b86, Python 3.13.1.

Proposed fix (optional)

Merge request_kwargs into kwargs in _call() instead of passing both. Pass the request-level subset of s3_additional_kwargs to the part, copy, complete, and abort requests of S3File and _copy_object_with_multipart_upload(). Unit tests with a mocked client can check the request parameters. A real-S3 SSE-C test depends on the bucket allowing SSE-C.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions