Centralize shared execute() kwargs into an ExecuteOptions dataclass#733
Draft
laughingman7743 wants to merge 4 commits into
Draft
Centralize shared execute() kwargs into an ExecuteOptions dataclass#733laughingman7743 wants to merge 4 commits into
laughingman7743 wants to merge 4 commits into
Conversation
Extract the execution arguments shared by all cursor implementations (work_group, s3_staging_dir, cache_size, cache_expiration_time, result_reuse_enable, result_reuse_minutes, paramstyle, on_start_query_execution, result_set_type_hints) into a frozen ExecuteOptions dataclass in pyathena/options.py. - BaseCursor._execute() and AioBaseCursor._execute() now take a single ExecuteOptions instead of nine individual kwargs - Every cursor's execute() keeps its existing keyword arguments for backward compatibility and additionally accepts a keyword-only options parameter; individual kwargs take precedence over options - Adding a new shared execute() argument now only requires changes to the dataclass and the internals, not all cursor signatures - Add the explicit result_set_type_hints argument to the aio pandas/arrow/polars/s3fs cursors, which previously relied on kwargs forwarding, for consistency with the other cursors - Export ExecuteOptions from the pyathena package root and document it in the usage guide and API reference Closes #691 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Simplify ExecuteOptions.merge() to rely on dataclasses.replace() for unknown-field validation and remove the unused logging boilerplate - Type cache_size/cache_expiration_time as plain int with default 0 and drop the duplicated None coercion in both _execute implementations - Add ExecuteOptions.resolve() and use it in all execute() methods instead of repeating the default-construction-and-merge expression - Invoke on_start_query_execution callbacks in the aio cursors, which wait for query completion like the synchronous cursors, and add the explicit on_start_query_execution argument to their execute() methods - Add missing execute() docstrings to AsyncPandasCursor and AsyncArrowCursor - Clarify cache_size/cache_expiration_time interplay in the ExecuteOptions docstring, scope the usage guide section to SQL cursors, and document that None keyword arguments do not reset options fields Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eview - Extract the connection-level and execute-level on_start_query_execution invocation, previously repeated in 10 cursor execute() methods, into BaseCursor._call_on_start_query_execution - Use ExecuteOptions.resolve() for the None fallback in both _execute implementations instead of an inline conditional Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WHAT
Extracts the
execute()arguments shared by all cursor implementations into a frozenExecuteOptionsdataclass (pyathena/options.py) and refactors the internal plumbing to pass it around instead of nine individual kwargs.BaseCursor._execute()/AioBaseCursor._execute()now take(operation, parameters, options). This is a private-API signature change; third-party code calling_execute()with the old kwargs needs updating.execute()keeps its existing keyword arguments unchanged (fully backward compatible) and additionally accepts a keyword-onlyoptions: ExecuteOptions | Noneparameter. When both are given, individual kwargs take precedence overoptionsfields;Nonekwargs are treated as "not specified" and do not resetoptionsfields.ExecuteOptions.resolve()is the single home for the options-plus-kwargs merge used by all 15execute()implementations, andBaseCursor._call_on_start_query_execution()centralizes the callback invocation previously copy-pasted per cursor.cache_size/cache_expiration_timesignature defaults change from0toNone; the effective default of0now lives on the dataclass, so behavior is identical.result_set_type_hintsargument they were missing (previously only reachable via**kwargsforwarding).on_start_query_execution(connection-level, execute-level kwarg, and viaoptions). UnlikeAsyncCursor, they wait for query completion, so the early-query-ID callback is meaningful there.AsyncCursor-based cursors continue to ignore it as documented.ExecuteOptionsis exported from the package root (from pyathena import ExecuteOptions) and documented in the usage guide and API reference.Tests:
tests/pyathena/test_options.py: unit tests for defaults, immutability, andmerge()/resolve()precedence (including falsy values likeFalse/0overriding).tests/pyathena/test_cursor.py/tests/pyathena/aio/test_cursor.py: integration tests running queries viaoptions=ExecuteOptions(...), verifying kwargs-over-options precedence and the new aio callback support end to end.tests/pyathena/test_cursor.py(105 passed), async cursor suite, all callback tests (14 passed), and per-cursor smoke tests (pandas/arrow/polars/s3fs, sync/async/aio) pass against real Athena.WHY
Closes #691.
Each new shared
execute()parameter (e.g.result_set_type_hintsin #690) currently requires editing 11+ cursor files with identical boilerplate. Centralizing the shared kwargs in one dataclass means a future parameter is added in 1-2 files (the dataclass plus the internal reference), gives all cursor types the same defaults/merge semantics, and provides a single documentation source for the shared arguments — while keeping every existing call site working unchanged.🤖 Generated with Claude Code