[laughingman7743]

WHAT

Implements steps 1–5 and 7 of #1363: the S3 filesystems take a boto3 S3 client instead of a PyAthena Connection.

• S3FileSystem and AioS3FileSystem take keyword-only client= and retry_config=.
  The filesystem borrows the client; close() does not close it.
  max_workers sizes the request pool but not the connection pool of a given client.
• pyathena.filesystem.s3_filesystem(connection, filesystem_class=S3FileSystem, **kwargs) creates a filesystem with connection.s3_client and connection.retry_config.
  The pandas, Polars, and s3fs result sets use it, and so does the filesystem benchmark.
  The s3fs result set still passes connection= to a filesystem_class other than S3FileSystem and AioS3FileSystem, including subclasses, until the next major version (step 6).
• connection= is deprecated.
  Passing it emits a DeprecationWarning.
  On Python 3.12+, the warning skips the frames of fsspec and the filesystem modules, so it is attributed to the caller, including through fsspec.filesystem() or a subclass.
  On 3.11, it is attributed to the direct caller of the constructor.
  As with any constructor-time warning, a call that fsspec's instance cache answers does not warn again.
• client= or connection= combined with an argument the filesystem uses to build a client raises TypeError.
  Those arguments are key/username, secret/password, token, profile/profile_name, anon, use_ssl, endpoint_url, connect_timeout, read_timeout, client_kwargs, config_kwargs, and the boto3 session arguments.
  The check runs before the connection's client is used, for both classes.
  connection= combined with client= or retry_config= also raises TypeError.
  A value of None counts as not given, and so does a value that fsspec's configuration supplies, such as FSSPEC_S3_ENDPOINT_URL.
• A filesystem built with client= or connection= raises TypeError from __reduce__, and so does a file it opened for reading.
  A file opened for writing still raises fsspec's RuntimeError.
  The message names the s3fs-style arguments.
  Filesystems built from s3fs-style arguments still pickle.
• The argument-name lists the filesystem uses to build its own client now live on S3FileSystem, so pyathena/filesystem/s3.py no longer imports Connection at runtime.
• docs/filesystem.md covers the three ways to create a filesystem, the argument restrictions, the deprecation, and a "Pickling and processes" section.
  docs/usage.md (including the checksum settings of a filesystem given client=) and docs/s3fs.md are updated, and the API reference lists s3_filesystem.

Release notes (next minor)

• Added client= and retry_config= to S3FileSystem and AioS3FileSystem, and added pyathena.filesystem.s3_filesystem().
• Deprecated connection= of S3FileSystem and AioS3FileSystem; it will be removed in the next major version.
  Use s3_filesystem(connection) or client=connection.s3_client, retry_config=connection.retry_config.
  In storage_options, replace {"connection": conn} with {"client": conn.s3_client, "retry_config": conn.retry_config}.
• Stricter validation: client=/connection= combined with a client-building argument raises TypeError where that argument used to be ignored silently.
  This includes anon=False and region_name, but not settings that come from fsspec's configuration.
• Pickling a filesystem built with client=/connection=, or one of the files it opened for reading, raises TypeError with guidance instead of a pickling error from botocore.

WHY

Closes #1363.
Fixes items 3 and 4 of #1339.
The filesystem's dependency on PyAthena's Connection is replaced with boto3-level injection, in preparation for a possible extraction of the filesystem.
The design was agreed in the issue.

TEST

Rebased onto d03e034, after #1374, #1393, and #1395.
The tests that #1395 added construct AioS3FileSystem with client=mock_client() instead of the deprecated connection=.
On ab4df45:

• just lint, just docs lint: pass.
• tests/pyathena/filesystem/test_s3.py, test_s3_async.py, test_s3_core.py, test_s3_executor.py, tests/pyathena/s3fs/test_result_set.py, tests/pyathena/pandas/test_result_set.py, tests/pyathena/polars/test_result_set.py, and tests/pyathena/test_connection.py with -W "error:The connection argument:DeprecationWarning": 1745 passed, 6 skipped.
• just benchmark test: 340 passed, 1 skipped.

Earlier runs:

The full local runs below were on 93f2459.
The later commits change the conflict check, the warning, tests, comments, and docs.
On ac578d7 (plus the PT011 fix in f712d7e), TestS3FileSystem, TestAioS3FileSystem, tests/pyathena/s3fs/test_result_set.py, tests/pyathena/pandas/test_result_set.py, tests/pyathena/polars/test_result_set.py, and tests/pyathena/test_connection.py were rerun with the deprecation filter: 1248 passed, 6 skipped.

• just format, just lint: pass.
• just docs lint: pass.
• Local runs used uv run --with 'botocore[crt]' --env-file .env pytest.
  -W "error:The connection argument:DeprecationWarning" was set so that any internal use of connection= fails:
  • tests/pyathena/filesystem tests/pyathena/s3fs tests/pyathena/pandas/test_result_set.py tests/pyathena/polars/test_result_set.py tests/pyathena/test_connection.py
    The first run found tests that patched result_set.S3FileSystem or passed storage_options={"connection": ...}; those were fixed. The filesystem tests passed in that run.
  • tests/pyathena/{pandas,polars,s3fs,arrow} tests/pyathena/aio/{pandas,polars,s3fs,arrow}: 912 passed, 8 skipped.
• just benchmark test: 340 passed, 1 skipped.
• New tests:
  • client= injection and its defaults
  • s3_filesystem() for both classes, with no warning
  • the deprecation warning and its location
  • every client-building argument with client= and connection=, and that the connection's client is not touched
  • connection= combined with client=/retry_config=
  • a request through an injected client retried by the injected RetryConfig (SlowDown, which the default policy does not retry), for both classes
  • warning attribution through fsspec.filesystem() (Python 3.12+)
  • pickling a filesystem and an open file with an injected client, and a pickle round trip from s3fs arguments
  • fsspec configuration values are ignored by the conflict check, while an explicit different value is rejected
  • the s3fs result set: no warning for PyAthena's classes, the deprecated path for a subclass
• Not run locally: the SQLAlchemy suites, which do not touch the filesystem's construction. CI runs the full suites after Ready.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round one (behavior, failure paths, resource boundaries, fsspec contracts, simplicity, test quality)

Base 7fcbcc64b02f9a5efe16ec04815b4143d1a2c615, head 93f2459d6ffc8146316ff15ab1aa81b48d64bd51.

The review covered:

• the S3FileSystem/AioS3FileSystem constructors: injection, conflicts, deprecation and stacklevel, __reduce__, close()
• s3_filesystem()
• the pandas metadata reader, the Polars CSV and chunked UNLOAD reads, and _create_s3_file_system in the s3fs result set
• fsspec's _Cached.__call__/apply_config/__reduce__, and AbstractBufferedFile.__reduce__
• the docs and the migrated and new tests

FINDINGS

1. fsspec's metaclass merges fsspec.config.conf["s3"], including the FSSPEC_S3_* environment variables, into the kwargs of every filesystem. With FSSPEC_S3_ENDPOINT_URL=http://localhost:9000, S3FileSystem(client=c) raised TypeError: endpoint_url cannot be combined ... (reproduced). Every pandas, Polars, or s3fs result set read would therefore fail for users who configure s3fs globally. Before this PR, the connection's client was used.
   Repair (b161e98): values that fsspec's configuration supplies are ignored by the check; an explicit argument with a different value is still rejected. The docs and docstring state this. Covered by test_client_ignores_fsspec_config.
2. Simplicity: test_pickle_s3fs_args read botocore's private _request_signer._credentials. That assertion was dropped; the region and block size still show the round trip.

No other findings. Checked and found correct:

• self._connection on the pandas metadata reader, which has no connection property
• an open rb file pickles its fs, so the TypeError propagates
• Aio converts connection itself, so exactly one warning is emitted, at the caller

Rerun: just lint, just docs lint, and the filesystem tests -k "client or s3_filesystem or deprecated or conflicts or rejected or pickle": 61 passed.

[laughingman7743]

Self-review round two (claims, existing callers, documentation reader, evidence)

Base 7fcbcc64b02f9a5efe16ec04815b4143d1a2c615, head b161e98b9843fce7984f254169cf76e68953573f. This was a full claim pass over the PR body, docstrings, docs, and commit messages.

Claims checked against code and dependency source:

• close() does not close the client: close() only releases _request_pool.
• max_workers does not resize a given client's pool: the client is used as is, and config_kwargs applies only to a client the filesystem builds.
• The warning points at the caller: stacklevel=3 steps past _Cached.__call__. Both classes assert record[0].filename == __file__, and Aio emits one warning.
• The check runs before the connection's client is used: asserted with a PropertyMock of s3_client.
• Filesystems built from s3fs arguments still pickle: round-trip tests for both classes.
• s3.py no longer imports Connection at runtime: the import is now under TYPE_CHECKING.
• Existing callers: positional S3FileSystem(connect()), subclasses that pass connection to super().__init__, and the s3fs filesystem_class user path all still work, with a warning. storage_options={\"connection\": ...} still works with a warning, and the release notes give the replacement.

FINDINGS

1. docs/filesystem.md and the PR body said that the files a client-injected filesystem opens raise TypeError when pickled. fsspec's AbstractBufferedFile.__reduce__ raises RuntimeError for write modes before the fs is reduced, so the claim holds only for files opened for reading.
   Repair (9d7d8eb): the docs, the comment, and the PR body now say "opened for reading". The PR body also records the fsspec-configuration exception from round one, and states which revision the full local runs used.

Documentation reader: the related docs (aio.md, s3fs.md, pandas.md, polars.md, usage.md) were searched for statements this change makes obsolete. The only one, the connection= sentence in usage.md, is already updated.

Evidence limits:

• The full local suites ran on 93f2459. After the repairs, only the targeted filesystem tests, lint, and docs lint were rerun.
• The SQLAlchemy suites were not run locally.
• AWS CI runs after Ready.

[laughingman7743]

Independent review (relayed): Codex CLI 0.166.0 (codex exec -s read-only), session 01a1b4a2-2267-7913-a4e0-1594f09a536d. Static review only: no edits, builds, tests, network, or GitHub access.

Base 7fcbcc64b02f9a5efe16ec04815b4143d1a2c615, head 9d7d8ebf4b40482a7f8be58bd1aeefa9f411f093. The package was a frozen detached snapshot plus the literal diff, the intended behavior, and docs/testing.md. It did not include the PR text, commit messages, or prior findings. The snapshot was not modified.

Coverage reported by the reviewer:

• the full diff: positional compatibility, subclasses, client ownership, argument lists
• fsspec configuration, instance cache and tokenization, open/url_to_fs, async construction, pickling
• the pandas, Polars, and s3fs result-set paths, custom filesystems, closed results
• the changed docs and all added or rewritten tests against docs/testing.md

FINDINGS (all P2):

1. s3.py:328: an explicit argument equal to the fsspec-configured value bypasses the conflict check.
   Declined. fsspec merges configuration into kwargs in its metaclass before __init__, and both calls get the same instance-cache token, so fsspec itself treats them as the same call. Provenance is not observable without replacing fsspec's metaclass. The docs state that configuration-supplied settings are ignored.
2. s3_async.py:176: AioS3FileSystem(connection=conn, profile=...) read conn.s3_client before the internal filesystem rejected profile. Verified.
   Repaired (ac578d7): both classes call S3FileSystem._resolve_injected_client(), which warns, rejects conflicts, and only then reads the connection's client. test_connection_conflicts_rejected (Aio) asserts that the s3_client property is not accessed.
3. s3.py:315 / s3_async.py:172: through fsspec.filesystem(\"s3\", connection=...) or a subclass, stacklevel=3 pointed into fsspec. Verified.
   Repaired (ac578d7): on Python 3.12+, skip_file_prefixes skips the fsspec and pyathena/filesystem frames. On 3.11, stacklevel=4 attributes the warning to the direct caller. test_connection_deprecated_through_fsspec covers this on 3.12+. A cache hit does not warn again: this is deferred as inherent to constructor-time warnings, and the PR body states it.
4. test_s3.py:2947 / test_s3_async.py:2022: the client tests compared the stored RetryConfig instead of observing its use. Verified.
   Repaired (ac578d7, f712d7e): the tests now stub SlowDown twice with RetryConfig(exceptions=[\"SlowDown\"], attempt=2, multiplier=0) and assert that both responses are consumed. The default policy does not retry SlowDown, so an ignored policy fails the test.

Self-review of the repair, narrow, via git range-diff 7fcbcc64..9d7d8ebf 7fcbcc64..f712d7e2 (commits 1–3 are unchanged):

• Round one (behavior):
  • The 3.11 frame count is user → _Cached.__call__ → __init__ → _resolve_injected_client, so the level is 4 for both classes.
  • On 3.12+, the s3fs result-set path for a user filesystem_class attributes the warning to pyathena/s3fs/result_set.py, which is outside the skipped prefixes.
  • The internal S3FileSystem of Aio receives client= without connection, so it does not warn twice.
• Round two (claims): the PR body now states the 3.11/3.12 attribution and the cache-hit limit. The docs make no attribution claim.
• Validation on ac578d7: just lint reported PT011 after the commit, which f712d7e fixes; lint now passes. TestS3FileSystem, TestAioS3FileSystem, and the s3fs/pandas/Polars result-set and connection tests: 1248 passed, 6 skipped. The client tests passed on f712d7e.

An independent follow-up on the repair is pending.

[laughingman7743]

Independent follow-up (relayed): Codex CLI 0.166.0 (codex exec -s read-only), session 01a1b4af-27da-70b1-b5d6-e49beedc0d24. Static review only.

Range: the patch 9d7d8ebf..f712d7e2 (head f712d7e2152f6c957e5d987e9c0b4f0a983d16a6), with the full diff from 7fcbcc64 supplied as context. The snapshot remained clean.

Coverage reported by the reviewer:

• the shared resolver in both constructors: conflicts are rejected before connection.s3_client is read, and the fsspec-config exemption and retry forwarding are kept
• skip_file_prefixes semantics on 3.12+, and stacklevel=4 on 3.11
• subclasses and the s3fs user filesystem_class path, where the warning lands at pyathena/s3fs/result_set.py:160, the actual constructor caller
• whether each changed test fails without its behavior: dropping the custom policy leaves one stubbed response pending; the property spy catches premature client access; the filename assertion catches misattribution

Result: CLEAN.

[laughingman7743]

Rebase record. The branch was rebased onto origin/master (29cc1d3d81 = merge-base) to resolve a conflict with #1369 in test_s3_async.py. The new head is d106826b405ff56eb87903f6fdb2780f8cefd1fc; the published head before the rebase was f712d7e2, and --force-with-lease checked against it.

git range-diff 7fcbcc64..f712d7e2 29cc1d3d..d106826b: commits 2–5 are identical. Commit 1 differs only in context. The resolved hunk keeps upstream's _object_exists stub and this PR's client=mock_client().

Upstream contracts inspected:

• Pass the source's SSE-C and RequestPayer parameters to the lookups of copies and setxattr #1333 (copy-source lookups), Keep the caller's parameters and the open-time pin in S3File #1334 (S3File parameters and pin), Fix recursive chmod, empty key segments, and version paths in listings #1335 (recursive chmod): none constructs a filesystem with connection= or changes __init__, __reduce__, or the argument lists.
• Close configuration gaps left by the rumdl migration #1368 is docs and lint configuration only.

Rerun on d106826b:

• just lint and just docs lint: pass.
• TestS3FileSystem, TestS3File, test_s3_async.py, and the s3fs/pandas/Polars result-set and connection tests with -W error:The connection argument:DeprecationWarning: 1394 passed, 6 skipped.

[laughingman7743]

Rebase record: rebased onto edfafcf59d4382c9af6d2111816ac1209c46dba7 (#1377) to resolve the conflict with S3FileSystem.__init__. New head be2487fba4193c53b4c46a4c01ff14b9eb102c19, previous head d106826b405ff56eb87903f6fdb2780f8cefd1fc.

• Conflict: the S3Core(...) call. Resolution keeps Send the authorization parameters of s3_additional_kwargs with every request #1377's request_kwargs (authorization parameters of s3_additional_kwargs plus their copy-source names) and this PR's retry_config default. git range-diff shows no other change to the patch series.
• Interaction checked: s3_additional_kwargs is not a client-building argument, so it stays allowed with client=, and its authorization parameters are sent with an injected client as with a built one. Send the authorization parameters of s3_additional_kwargs with every request #1377's new tests build filesystems from s3fs-style arguments, not connection=. The auto-merged docs/filesystem.md reads consistently.
• Validation: just lint passed; uv run --with 'botocore[crt]' --env-file .env pytest -n 4 -W "error:The connection argument:DeprecationWarning" tests/pyathena/filesystem tests/pyathena/s3fs tests/pyathena/test_connection.py: 1908 passed, 6 skipped. Result-set and SQLAlchemy suites not rerun locally; current CI covers them.