Repository navigation
Decide request-parameter coverage, error translation, and S3 retry codes #1342
Description
Part of #1327 (design problems 6 and 7 of the audit report).
Summary
Decide and document which request parameters reach which requests, how S3 errors are translated, and which layer retries S3 errors.
Current state (766d79d)
s3_additional_kwargsreachesopen,pipe_fileandput_fileonly (documented), nottouch,cat_file,cp_file/copy/mv,setxattrorrm: users migrating from s3fs who put SSE or SSE-C settings there get different results fromtouchorcpthan frompipe, andcat(p)fails whereopen(p).read()works. Per call,rm_fileignores parameters thatrmforwards,mkdirdrops everything butaclandregion_name, andtouch(path, Body=b"x")writes data although it creates an empty object.- Append mode updates the request with
appended.to_api_repr(), which overrides the caller'sContentType,StorageClassandMetadataand forwards SSE and object-lock fields of the existing object (s3.py:2803); documented as keeping the old object's metadata. - Error translation: only
mkdirturnsParamValidationErrorintoValueError;cat_fileandclear_multipart_uploadsre-parsee.__cause__because the translatedOSErrorcarries no S3 code;exists()maps 403 toTruefor buckets and raisesPermissionErrorfor keys;copy_object_annotationreads its payload without closing it or translating errors. - Retries: the default
RetryConfigretriesThrottlingExceptionandTooManyRequestsException, which are Athena's codes. S3'sSlowDown,ServiceUnavailable,InternalErrorandRequestTimeoutare retried only by botocore, sostop_retries_onandunstoppable_retriesgovern no S3 retry unless the user adds S3 codes to a policy that a connection shares with Athena.docs/filesystem.mdsays only that requests are "sent with the retry policy". Retrying in both layers is acceptable; the question is which codes PyAthena's layer retries for S3 and how that is documented.
Proposal (for decision)
- Document, or extend, the coverage of
s3_additional_kwargs, and makerm_file,mkdirandtouchconsistent with it. - Let explicit caller parameters win over the appended object's in append mode, or document the precedence.
- Carry the S3 error code on translated errors, add core options such as
missing_okfor aborts and anInvalidRangemode forget_object, and pick oneParamValidationErrorpolicy. - Give S3 its own default retry codes, separate from Athena's, or document that botocore owns S3 retries.
Activity
Revert the parameters of the expansion and pairing of copy and mv (GH…
laughingman7743 commentedon Nov 7, 2026
Input for item 1 from #1333 / #1362 (verified offline at 2a60ecc and at the PR heads).
Copy and move: the expansion and pairing requests send no parameters
#1362 makes the requests that read a copy's source send the source's authorization parameters: the source lookup of
_copy_file,mv's conflict lookup and deletion, andsetxattr's HeadObject. Before those requests,copy/mvwith string paths expand and pair the paths, and those requests send nothing. In a requester-pays bucket, or for an SSE-C source, acopy/mvtherefore still fails before the fixed lookups run. This holds even for a single file:copy("s3://b/a", "s3://b/b", RequestPayer="requester")sendsHeadObject(a),HeadObject(b)andListObjectsV2(b/)withoutRequestPayer. Lists of both sources and destinations are paired without these requests.docs/filesystem.mdstates both facts.The requests involved:
expand_path→glob/findlistings, and theexists(p)of a version path in a recursive copy.expand_path, which callsself.exists(p)without kwargs after an empty recursivefind._copy_pairs/fsspeccopy:isdir(p)).Why the straightforward fix was reverted
87ef858 (reverted in #1362) let
_ls_dirs/_find/find/expand_pathhonor the lookup parameters. It sent the source parameters to the source expansion and the destination's lookup parameters to the destination check, routing a parameterizedcopythrough_copy_pairs. The independent review found:copy()passes the CopyObject kwargs toexpand_path→find. Oncefindhonors lookup names, the destination'sExpectedBucketOwneris sent with the source listing. Example:copy("s3://src/data/*", ["s3://dst/r"], ExpectedSourceBucketOwner=A, ExpectedBucketOwner=B), wherepath2is a list, so the copy is not routed.reports/instead ofreports/2026-), which a prefix-scoped IAM policy denies._expand_pathrejects the extra keywords withTypeError.Design questions for this item:
ls/find/glob/walk/du) accept lookup parameters at all? If they do, by which channel, so that fsspec's forwarding of the CopyObject kwargs cannot leak destination-side names into source requests?copy/mvexpand with its own code (sync and aio) instead of fsspec's, keeping the aio glob prefix?laughingman7743 commentedon Nov 7, 2026
Decisions (2026-11-07), after an offline audit at 29cc1d3. Every item still held at that commit. Each sub-issue has the current state and the decision:
s3_additional_kwargs(RequestPayer,ExpectedBucketOwner,SSECustomer*), listings included, filtered per operation. This covers the copy/mv expansion above and what remains of Pass the source's SSE-C and RequestPayer parameters to the lookups of copies and setxattr #1333. The same PR fixes the per-call gaps ofrm_file,mkdir, andtouch.ParamValidationError→ValueErroreverywhere (breaking).S3ClientError.code_ofis documented as the public way to get the S3 code.exists()on a 403 stays as it is and is documented.copy_object_annotationcloses and translates its payload read. Coremissing_ok/InvalidRangeoptions are deferred.One PR per sub-issue, starting with #1372.