Skip to content

Use the connection's s3_config and session credentials in to_sql() - #1068

Merged
laughingman7743 merged 4 commits into
masterfrom
fix/1067-to-sql-s3-settings
Oct 4, 2026
Merged

laughingman7743 merged 4 commits into
masterfrom
fix/1067-to-sql-s3-settings

Conversation

@laughingman7743

@laughingman7743 laughingman7743 commented Oct 4, 2026 •

Copy link
Copy Markdown
Member

WHAT

pyathena.pandas.util.to_sql() now sends its S3 requests with the connection's botocore settings, and its upload workers use the credentials of a session given to connect().

  • The bucket resource (used for if_exists="replace") and the upload workers' resources (to_parquet()) get config=conn.s3_config, the connection's config merged with s3_config and PyAthena's user agent (Share one S3 client per connection across result sets and filesystems #1058). Before, they were built without a config, so proxies, timeouts, retries, max_pool_connections and the user agent did not apply.
  • The upload workers still resolve the credentials themselves from conn._session_kwargs and profile_name, so refreshable credentials (assume-role profiles, instance and container roles) stay refreshable. Only for a connection given its session (connect(session=...), recorded in the new private Connection._session_given), whose credentials those arguments cannot reproduce, the workers get that session's credentials as of the start of the uploads. Before, such workers used the default credential chain while the bucket resource used the connection's session.
  • No credentials are added when explicit keys are among the client arguments (they take precedence in the workers' resources anyway), or when a botocore_session is among the session arguments; both are checked by value, since SQLAlchemy passes aws_access_key_id=None without credentials in the URL. For a botocore session, boto3.Session(botocore_session=bs, aws_access_key_id=...) would set static credentials on the user's botocore session (checked locally).
  • The new worker arguments are a botocore.config.Config and strings, which pickle for ProcessPoolExecutor. A config with signature_version=UNSIGNED does not pickle; unsigned requests cannot write a table's data anyway.
  • Docstring of to_sql() and docs/pandas.md state which settings and credentials the S3 requests use.

Behavior changes (release-note candidates):

  • to_sql()'s S3 requests now use the connection's config/s3_config, including its retry settings and proxies.
  • With connect(session=...), to_sql() uploads with that session's credentials instead of the default credential chain.
  • With connect(session=...), the upload credentials are resolved once, when the uploads start; an upload that outlives temporary credentials of that session fails instead of refreshing. Other connections are unaffected.

Not changed: the workers still build their own session and resource per chunk instead of sharing Connection.s3_client. A botocore_session among the connection arguments is still deep-copied for the workers, which fails for one holding refreshable credentials and cannot be pickled for processes (pre-existing).

WHY

Fixes #1067, a follow-up of #1058.

TEST

Tested commit ab57b06 (base 911492c). AWS CI on ab57b06 (run 37178922657): test, test-sqla and test-sqla-async passed.

  • just lint, just docs lint: passed.
  • Live, against the CI test account: uv run --env-file .env pytest -n 2 tests/pyathena/pandas/test_util.py: 16 passed.
  • New test_to_sql_session_credentials_and_s3_config, for ThreadPoolExecutor and a ProcessPoolExecutor with the spawn start method (so that no forkserver keeps the test's invalid environment for later pools), and a thread pool case with aws_access_key_id=None, aws_secret_access_key=None as SQLAlchemy passes them: the connection gets a session built from the resolved credentials and s3_config=Config(max_pool_connections=37), and the environment then holds invalid keys, so the default credential chain fails. Without the change, the cases fail with InvalidAccessKeyId on PutObject (the None case also fails with the key-presence check of 4b22102); with it, all pass, and every Session.resource() call recorded in the test process (the bucket resource, and with threads the workers' resources) has max_pool_connections == 37.
  • New test_to_sql_workers_resolve_credentials: for a connection built without a session, the workers' session arguments carry no credentials. It guards the refresh behavior; it passes on master as well.
  • Not tested: an upload outliving temporary credentials.

🤖 Generated with Claude Code

to_sql() built its S3 resources without the connection's botocore
config, so proxies, timeouts, retries and the user agent did not apply,
and its upload workers built sessions from the connection's keyword
arguments only, so the credentials of connect(session=...) were ignored
and the uploads used the default credential chain. The resources now
get s3_config, and the workers get the frozen credentials of the
connection's session, which stay picklable for a ProcessPoolExecutor.

Closes #1067

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread pyathena/pandas/util.py Outdated
session_kwargs = deepcopy(conn._session_kwargs)
session_kwargs.update({"profile_name": conn.profile_name})
credentials = (
None if "botocore_session" in session_kwargs else conn.session.get_credentials()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round one (behavior and implementation): FINDINGS (1, repaired)

Base 911492c, head d540ea5 (full diff: pyathena/pandas/util.py, tests/pyathena/pandas/test_util.py, docs/pandas.md).

Covered:

  • Every S3 request of to_sql(): the bucket resource (objects.filter().delete() for if_exists="replace") and to_parquet()'s per-chunk Session(**session_kwargs).resource("s3", **client_kwargs). Both now get config=conn.s3_config.
  • Credentials: _session_kwargs keeps explicit keys and the role_arn/serial_number temporary keys (stored in Connection._kwargs); frozen credentials of conn.session have the same values in those cases and add the connect(session=...) case. profile_name is still passed, so profile settings other than credentials still apply. A session without credentials adds nothing (previous behavior).
  • Executors: the arguments are a Config and strings; Config pickles (checked), and the live test passes with ProcessPoolExecutor.
  • Test quality: the test makes the default credential chain invalid after building the connection's session, so it fails on the original source for both executors (InvalidAccessKeyId on PutObject) and checks max_pool_connections on every resource recorded in the test process.

Finding: with connect(botocore_session=bs), _session_kwargs contains bs, and boto3.Session(botocore_session=bs, aws_access_key_id=...) calls bs.set_credentials(), replacing the user's (possibly refreshable) credentials with static ones (checked locally: the key changed to the new one). Repaired in d764a59: no credentials are added when a botocore session is given; it already carries them. Pre-existing and unchanged: deepcopy(conn._session_kwargs) copies that botocore session, and a botocore session cannot be pickled for a ProcessPoolExecutor.

Comment thread pyathena/pandas/util.py
)
client_kwargs = deepcopy(conn._s3_client_kwargs)
client_kwargs.update({"region_name": conn.region_name})
client_kwargs.update({"region_name": conn.region_name, "config": conn.s3_config})

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round two (claims, callers, operations): CLEAN

Base 911492c, head d764a59 (PR body, commit messages, docstring, docs/pandas.md, issue #1067).

  • "Proxies, timeouts, retries, max_pool_connections and the user agent did not apply": master's conn.session.resource("s3", region_name=..., **conn._s3_client_kwargs) and session.resource("s3", **client_kwargs) pass no config, and _client_kwargs never contains config (it is a named Connection parameter).
  • "The uploads used the default credential chain with connect(session=...)": reproduced in to_sql() ignores the connection's botocore config and the credentials of connect(session=...) #1067 (worker session_kwargs == {'profile_name': None}), and the live test fails on master with the environment's invalid keys.
  • "Credentials resolved once": get_frozen_credentials() is called once per to_sql() call before the chunks are submitted; before, each chunk's new session resolved them again. Stated as a behavior change, with the role_arn/serial_number precedent (fixed temporary keys in _kwargs).
  • Existing callers: to_sql() and to_parquet() signatures are unchanged; to_parquet() callers that pass their own kwargs are unaffected.
  • AWS operator: the S3 requests now follow the connection's retry config inside botocore in addition to retry_api_call around PutObject, as the result-set requests already do.
  • Docs: the docs/pandas.md sentence links to the existing "S3 client" section; just docs lint passed.

Freezing the credentials for every connection made refreshable
credentials (assume-role profiles, instance and container roles) expire
in a long upload, and resolved the session's credentials even when
explicit keys take precedence. The workers now resolve the credentials
themselves as before, unless the connection was given its session; then
they get its credentials as of the start of the uploads. Connection
records whether its session was given.

The process pool test uses the spawn start method, so that no
forkserver keeps the invalid keys of its environment for later pools.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread pyathena/pandas/util.py
session_kwargs = deepcopy(conn._session_kwargs)
session_kwargs.update({"profile_name": conn.profile_name})
if (
conn._session_given

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review (relayed): FINDINGS, static review.

  • Reviewer: OpenAI Codex CLI 0.160.0, model gpt-6-astra, effort high, sandbox read-only, session 01a10536-d1ce-7181-8df3-a4a8e53134f8. Scope 911492c3..d764a591 in a detached snapshot without .env; no PR text or earlier findings in the prompt; snapshot unchanged afterwards.
  • Covered (reviewer's list): Connection session construction, credential precedence, _session_kwargs/_s3_client_kwargs/config merging; replacement listing/deletion, partitioned and unpartitioned uploads, worker serialization, retries; both executors, supplied-session side effects, refreshable/missing credentials, tests, docstring and docs; boto3/botocore 1.43.102 and multiprocessing/pickle sources.

Findings and dispositions:

  1. (Introduced, P2) Freezing credentials for every connection made the workers' refreshable credentials (assume-role profiles, instance/container roles) expire in a long upload. Verified; redesigned in 4b22102 after the maintainer chose to freeze only for connect(session=...): Connection._session_given records a given session; otherwise the workers resolve credentials themselves as before. test_to_sql_workers_resolve_credentials checks that a connection without a session passes no credentials to the workers.
  2. (Introduced, P2) s3_config=Config(signature_version=UNSIGNED) cannot be pickled for ProcessPoolExecutor. Verified; not changed: unsigned requests cannot write a table's data, so this configuration cannot work with to_sql() anyway. Stated in the PR body.
  3. (Introduced, P2) The session's credentials were resolved even when explicit keys take precedence, so a partial-credentials environment could raise PartialCredentialsError. Repaired in 4b22102: no lookup when aws_access_key_id is among the client arguments.
  4. (Introduced, P2) Under the forkserver start method (Linux default since Python 3.14), the process-pool test could start a forkserver that keeps the test's invalid keys for later pools. Repaired in 4b22102: the test uses a ProcessPoolExecutor with the spawn context, whose workers exit with the pool.
  5. (Pre-existing) A botocore_session among the connection arguments is deep-copied for the workers, which fails for one holding refreshable credentials and cannot be pickled for processes. Not changed; the code comment no longer claims the arguments are picklable, and the PR body lists it under "Not changed".

Comment thread pyathena/connection.py

# Whether the session was given rather than built from the arguments,
# which then do not reproduce its credentials.
self._session_given = bool(session)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review of the repair (both perspectives), scope d764a5917a96e37c67b1de89c64ab79254cc7a8e..4b221020a1139834d813876791f89d38d4955059: CLEAN

Round one (behavior): _session_given is bool(session), matching the if session: branch that uses it. The freeze condition requires a given session, no explicit client keys, no botocore session, and non-empty credentials; every other connection keeps master's worker arguments, so refresh behavior is unchanged there. test_to_sql_session_credentials_and_s3_config (thread and spawn process pools) fails on master's source and passes; test_to_sql_workers_resolve_credentials passes (it would fail on d764a59, which froze for every connection). tests/pyathena/pandas/test_util.py: 15 passed; just lint passed.

Round two (claims): the docstring and docs/pandas.md now say the workers resolve credentials themselves except for a given session; the PR body's behavior changes limit the once-at-start resolution to connect(session=...). Connection._session_given is private and only read by to_sql(). just docs lint passed.

SQLAlchemy passes aws_access_key_id=None without credentials in the URL,
and None does not take precedence over the given session's credentials.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread pyathena/pandas/util.py
session_kwargs.update({"profile_name": conn.profile_name})
if (
conn._session_given
and not conn._s3_client_kwargs.get("aws_access_key_id")

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent follow-up review (relayed): FINDINGS, static review.

  • Reviewer: OpenAI Codex CLI 0.160.0, model gpt-6-astra, effort high, sandbox read-only, session 01a10541-28d5-7932-8526-98cd168c702a. Scope d764a591..4b221020 (the redesign) in a detached snapshot without .env; snapshot unchanged afterwards.
  • Covered (reviewer's list): default/profile credentials, explicit keys, assume-role/MFA, supplied boto3/botocore sessions, both executors; cleanup and upload S3 configuration incl. partitions; tests, environment restoration, process isolation, docstrings and docs; boto3/botocore 1.43.102. No test environment or monkeypatch leak found; the spawn context avoids the forkserver problem.

Findings and dispositions:

  1. (Introduced, P2) The key-presence check skipped forwarding for aws_access_key_id=None, which botocore treats as unset; SQLAlchemy's create_connect_args() always passes it (None without credentials in the URL). Verified and repaired in ab57b06: both checks look at values. New test case [ThreadPoolExecutor-connect_kwargs2] (aws_access_key_id=None, aws_secret_access_key=None) fails with InvalidAccessKeyId on PutObject with the 4b22102 check and passes now.
  2. (Introduced in d764a59, P2) botocore_session=None likewise suppressed forwarding. Repaired in ab57b06 by the same value check.
  3. (Pre-existing) A botocore session holding refreshable credentials cannot be deep-copied for the workers. Not changed; listed under "Not changed" in the PR body.
  4. Docs wording omitted the explicit-key precedence. Repaired in ab57b06 (docstring and docs/pandas.md: "a session and no explicit keys").

After the repair: just lint, just docs lint, tests/pyathena/pandas/test_util.py: 16 passed.

Comment thread pyathena/pandas/util.py
if (
conn._session_given
and not conn._s3_client_kwargs.get("aws_access_key_id")
and not session_kwargs.get("botocore_session")

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent follow-up review of ab57b06 (relayed): CLEAN, static review.

  • Reviewer: OpenAI Codex CLI 0.160.0, model gpt-6-astra, effort high, sandbox read-only, session 01a1054a-3c5e-7ce2-b5eb-c951416abdf9. Scope 4b221020..ab57b063 in a detached snapshot without .env; snapshot unchanged afterwards.
  • Result: "No actionable defects introduced by this commit. The value checks handle SQLAlchemy's None credential arguments, and the new test parameter detects the original failure. The wording change introduces no new inaccuracy."

@laughingman7743
laughingman7743 marked this pull request as ready for review October 4, 2026 05:06
@laughingman7743
laughingman7743 merged commit 74cdab5 into master Oct 4, 2026
14 checks passed
@laughingman7743
laughingman7743 deleted the fix/1067-to-sql-s3-settings branch October 4, 2026 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

to_sql() ignores the connection's botocore config and the credentials of connect(session=...)

1 participant