You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Name-mangled (__name) helper methods hide shared behavior from subclasses. Because a subclass cannot override or reuse them, each class that needs the behavior keeps its own copy, and the copies drift.
PyAthena used few of them, but the count has grown recently.
Counting non-dunder def __name( in pyathena/:
Open PR #853 would add eight more and remove one: __start_query_execution, __stop_started_query, __cancel_and_wait and __stop_started_calculation, each in a sync and an async version.
The other mangled helpers on master:
__poll in BaseCursor (pyathena/common.py), AioBaseCursor (pyathena/aio/common.py), SparkBaseCursor (pyathena/spark/common.py) and AioSparkCursor (pyathena/aio/spark/cursor.py). The four have almost the same body: poll the status, call on_poll, stop at a terminal state, sleep. Each subclass also has to override _poll because it cannot reach the parent's __poll.
AthenaResultSet.__get_query_results and __fetch (pyathena/result_set.py), and AthenaAioResultSet.__async_get_query_results and __async_fetch (pyathena/aio/result_set.py).
__s3_file_system in AthenaArrowResultSet and AthenaPandasResultSet. The two bodies are different: the Arrow one returns pyarrow's S3FileSystem, the pandas one returns PyAthena's. AthenaS3FSResultSet names the same role _create_s3_file_system.
Neither the tests nor the package access a mangled name from outside its class (no _ClassName__name references), so renaming them is internal.
Proposed change
Replace the mangled helpers with single-underscore methods on the class that owns the behavior. Do not add new __name helpers.
Share logic that is the same between sync and async, or between the SQL and Spark cursors, through single-underscore helpers on the base class instead of per-class copies. Start with the Spark helpers from Terminate a newly started Spark session when cursor setup fails #832 and Cancel a Spark calculation interrupted while it is being started #861, and with the four __poll loops.
Where the sync and async bodies differ only in await and the sleep call, keep the non-I/O parts shared (terminal states, logging, recording the final execution). Keep separate sync and async loops where sharing would need a larger abstraction.
Make the result-set helper names consistent (__s3_file_system versus _create_s3_file_system).
Keep the signatures of the existing single-underscore methods that downstream subclasses call (_execute, _poll, _cancel, _get_query_execution, _find_previous_query_id; see the v3.35.1 hotfix for Backwards incompatible changes in 3.35 breaks dbt projects #734). New single-underscore names become visible to subclasses, so pick names that are unlikely to collide with a subclass's own names.
The affected PyAthena tests on AWS: tests/pyathena/test_cursor.py, tests/pyathena/aio/test_cursor.py, tests/pyathena/spark/, tests/pyathena/aio/spark/ (existing interrupt, cancel and session-cleanup tests), and the result-set tests of the Arrow, pandas, Polars and S3FS cursors.
Re-count the mangled helpers, and diff the remaining polling loops, to show the duplication was removed.
Use case
Name-mangled (
__name) helper methods hide shared behavior from subclasses. Because a subclass cannot override or reuse them, each class that needs the behavior keeps its own copy, and the copies drift.PyAthena used few of them, but the count has grown recently.
Counting non-dunder
def __name(inpyathena/:All six added since v3.36.0 come from the Spark lifecycle work in #791:
SparkBaseCursor.__terminate_session(Terminate a newly started Spark session when cursor setup fails #832)SparkBaseCursor.__start_calculation,__cancel_and_wait, and__wait_for_start, plusAioSparkCursor.__start_calculationand__cancel_and_wait(Cancel a Spark calculation interrupted while it is being started #861)Open PR #853 would add eight more and remove one:
__start_query_execution,__stop_started_query,__cancel_and_waitand__stop_started_calculation, each in a sync and an async version.The other mangled helpers on master:
__pollinBaseCursor(pyathena/common.py),AioBaseCursor(pyathena/aio/common.py),SparkBaseCursor(pyathena/spark/common.py) andAioSparkCursor(pyathena/aio/spark/cursor.py). The four have almost the same body: poll the status, callon_poll, stop at a terminal state, sleep. Each subclass also has to override_pollbecause it cannot reach the parent's__poll.AthenaResultSet.__get_query_resultsand__fetch(pyathena/result_set.py), andAthenaAioResultSet.__async_get_query_resultsand__async_fetch(pyathena/aio/result_set.py).__s3_file_systeminAthenaArrowResultSetandAthenaPandasResultSet. The two bodies are different: the Arrow one returns pyarrow'sS3FileSystem, the pandas one returns PyAthena's.AthenaS3FSResultSetnames the same role_create_s3_file_system.Neither the tests nor the package access a mangled name from outside its class (no
_ClassName__namereferences), so renaming them is internal.Proposed change
__namehelpers.__pollloops.Where the sync and async bodies differ only in
awaitand the sleep call, keep the non-I/O parts shared (terminal states, logging, recording the final execution). Keep separate sync and async loops where sharing would need a larger abstraction.__s3_file_systemversus_create_s3_file_system)._execute,_poll,_cancel,_get_query_execution,_find_previous_query_id; see the v3.35.1 hotfix for Backwards incompatible changes in 3.35 breaks dbt projects #734). New single-underscore names become visible to subclasses, so pick names that are unlikely to collide with a subclass's own names.Validation plan (if implementing)
just lint.tests/pyathena/test_cursor.py,tests/pyathena/aio/test_cursor.py,tests/pyathena/spark/,tests/pyathena/aio/spark/(existing interrupt, cancel and session-cleanup tests), and the result-set tests of the Arrow, pandas, Polars and S3FS cursors.