Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions pyathena/aio/sqlalchemy/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
_ASYNC_CURSOR_CLASSES: dict[Any, Any] = {Cursor: AioCursor}


class AsyncAdapt_pyathena_cursor:
class AthenaAioAdaptedCursor:

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 (implementation behavior): CLEAN

Scope: git diff a667abd4a8f8d91ff6b68fd8e3b49e12fe8f0a0b..14f636c3a7858ead1201a6d5894ea01e1923bae5 (pyathena/aio/sqlalchemy/base.py, pyproject.toml, tests/pyathena/aio/sqlalchemy/test_base.py, tests/pyathena/sqlalchemy/test_base.py).

  • Behavior: pure rename of the three adapter classes; bodies, __slots__, and the import_dbapi()/dbapi() return objects are unchanged. SQLAlchemy receives the DBAPI object from import_dbapi() and never looks the classes up by name; git grep -i asyncadapt finds no string-based or dynamic lookup, __all__, or Sphinx autoclass entry for them (only SQLAlchemy's own pool.AsyncAdaptedQueuePool remains).
  • Framework contracts: the awsathena+aio* entry points in pyproject.toml:68-72 name the AthenaAio*Dialect classes, not the adapters.
  • Lint: removing the N801 exclusion leaves just lint green, and ruff check --select N801 rejects a temporary class AsyncAdapt_pyathena_cursor: under pyathena/aio/sqlalchemy/. The N802 exclusion for compiler.py stays as required.
  • Tests: references and the test class name (TestAthenaAioAdaptedCursor) follow the rename; no new behavior needs new coverage. tests/pyathena/aio/sqlalchemy/ (53 passed) and just test sqla-async (589 passed, 759 skipped) exercise the adapters end to end.

Findings: none.

"""Wraps any async PyAthena cursor with a sync DBAPI interface.

SQLAlchemy's async engine uses greenlet-based ``await_only()`` to call
Expand Down Expand Up @@ -221,24 +221,24 @@ def list_table_metadata(self, *args: Any, **kwargs: Any) -> Any:
"""
return await_only(self._cursor.list_table_metadata(*args, **kwargs))

def __enter__(self) -> AsyncAdapt_pyathena_cursor:
def __enter__(self) -> AthenaAioAdaptedCursor:
return self

def __exit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None:
self.close()


class AsyncAdapt_pyathena_connection(AdaptedConnection):
class AthenaAioAdaptedConnection(AdaptedConnection):
"""Wraps ``AioConnection`` with a sync DBAPI interface.

This adapted connection delegates ``cursor()`` to the underlying
``AioConnection`` and wraps each returned async cursor with
``AsyncAdapt_pyathena_cursor``.
``AthenaAioAdaptedCursor``.
"""

__slots__ = ("_connection", "dbapi")

def __init__(self, dbapi: AsyncAdapt_pyathena_dbapi, connection: AioConnection) -> None:
def __init__(self, dbapi: AthenaAioAdaptedDBAPI, connection: AioConnection) -> None:
"""Initialize the adapted connection.

Args:
Expand Down Expand Up @@ -273,7 +273,7 @@ def retry_config(self) -> RetryConfig:
"""The retry configuration of the wrapped connection."""
return self._connection.retry_config # type: ignore[no-any-return]

def cursor(self, cursor: Any = None, **kwargs: Any) -> AsyncAdapt_pyathena_cursor:
def cursor(self, cursor: Any = None, **kwargs: Any) -> AthenaAioAdaptedCursor:
"""Create an async cursor on the wrapped connection and adapt it.

A synchronous cursor class that has a registered async counterpart is
Expand All @@ -286,14 +286,14 @@ def cursor(self, cursor: Any = None, **kwargs: Any) -> AsyncAdapt_pyathena_curso
``cursor()``.

Returns:
The created cursor wrapped in ``AsyncAdapt_pyathena_cursor``.
The created cursor wrapped in ``AthenaAioAdaptedCursor``.
"""
# The shared dialect names a cursor class in its synchronous form; this
# connection can only drive the async counterpart.
raw_cursor = self._connection.cursor(_ASYNC_CURSOR_CLASSES.get(cursor, cursor), **kwargs)
return AsyncAdapt_pyathena_cursor(raw_cursor)
return AthenaAioAdaptedCursor(raw_cursor)

def _internal_cursor(self, cursor: Any) -> AsyncAdapt_pyathena_cursor:
def _internal_cursor(self, cursor: Any) -> AthenaAioAdaptedCursor:
"""Create a cursor with the wrapped connection's ``_internal_cursor()`` and adapt it.

A synchronous cursor class is replaced as in ``cursor()``.
Expand All @@ -302,10 +302,10 @@ def _internal_cursor(self, cursor: Any) -> AsyncAdapt_pyathena_cursor:
cursor: The cursor class to create.

Returns:
The created cursor wrapped in ``AsyncAdapt_pyathena_cursor``.
The created cursor wrapped in ``AthenaAioAdaptedCursor``.
"""
raw_cursor = self._connection._internal_cursor(_ASYNC_CURSOR_CLASSES.get(cursor, cursor))
return AsyncAdapt_pyathena_cursor(raw_cursor)
return AthenaAioAdaptedCursor(raw_cursor)

def close(self) -> None:
"""Close the wrapped connection."""
Expand All @@ -319,7 +319,7 @@ def rollback(self) -> None:
"""Do nothing, because Athena does not support transactions."""


class AsyncAdapt_pyathena_dbapi:
class AthenaAioAdaptedDBAPI:

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 (compatibility, operations, factual claims): CLEAN

Scope: git diff a667abd4a8f8d91ff6b68fd8e3b49e12fe8f0a0b..14f636c3a7858ead1201a6d5894ea01e1923bae5, plus the PR description.

  • Compatibility: code importing the old names from pyathena.aio.sqlalchemy.base breaks, with no alias, as Rename the asyncio SQLAlchemy adapter classes to CapWords and enforce N801 #1147 decided; the PR's release-note section states it. gh search code AsyncAdapt_pyathena outside this repo finds only vendored copies of PyAthena and package-metadata dumps, no importer. 3.x is not affected (master only).
  • Upstream drift: master advanced to 0ebc6e7 (Resume benchmark fleet jobs after Spot interruptions #1145, benchmarks only). A temporary merge of the head with that master passes ruff check . including N801, so the removed exclusion does not reject any newly merged class name.
  • Operations: pyproject.toml is a shared path in test.yaml, so marking Ready runs the full AWS suites; no AWS resources, permissions, or request patterns change.
  • Claims: the old→new table, the acronym convention (AthenaDDLCompiler, DBAPITypeObject), the N802 rationale, the entry-point claim, and the recorded test commands/results match the tree and the local runs at 14f636c.

Findings: none.

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.

Round-two addendum (issue premise check): in SQLAlchemy 2.0.46 (the locked version), engine/create.py, engine/default.py, engine/interfaces.py, pool/base.py, and connectors/asyncio.py use __class__.__name__ only in log and error messages for the dialect, pool, and exception classes, never to locate the DBAPI adapter. The renamed classes do not subclass SQLAlchemy's AsyncAdapt_dbapi_* bases (only the connection subclasses AdaptedConnection), so no inherited name-based behavior changes. Round two remains CLEAN.

"""Fake DBAPI module for the async SQLAlchemy engine.

SQLAlchemy expects ``import_dbapi()`` to return a module-like object
Expand All @@ -344,17 +344,17 @@ class AsyncAdapt_pyathena_dbapi:
DataError = DataError
NotSupportedError = NotSupportedError

def connect(self, **kwargs: Any) -> AsyncAdapt_pyathena_connection:
def connect(self, **kwargs: Any) -> AthenaAioAdaptedConnection:
"""Create an ``AioConnection`` and wrap it in an adapted connection.

Args:
**kwargs: Keyword arguments forwarded to ``AioConnection.create()``.

Returns:
The new connection wrapped in ``AsyncAdapt_pyathena_connection``.
The new connection wrapped in ``AthenaAioAdaptedConnection``.
"""
connection = await_only(AioConnection.create(**kwargs))
return AsyncAdapt_pyathena_connection(self, connection)
return AthenaAioAdaptedConnection(self, connection)


class AthenaAioDialect(AthenaDialect):
Expand Down Expand Up @@ -384,12 +384,12 @@ def get_pool_class(cls, url: URL) -> type:
@classmethod
@override
def import_dbapi(cls) -> ModuleType:
return AsyncAdapt_pyathena_dbapi() # type: ignore[return-value]
return AthenaAioAdaptedDBAPI() # type: ignore[return-value]

@classmethod
@override
def dbapi(cls) -> ModuleType: # type: ignore[override]
return AsyncAdapt_pyathena_dbapi() # type: ignore[return-value]
return AthenaAioAdaptedDBAPI() # type: ignore[return-value]

@override
def create_connect_args(self, url: URL) -> tuple[tuple[str], MutableMapping[str, Any]]:
Expand Down
3 changes: 0 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,6 @@ ignore-decorators = ["pyathena.util.override"]
"pyathena/sqlalchemy/compiler.py" = [
"N802", # SQLAlchemy TypeCompiler requires visit_UPPERCASE method names
]
"pyathena/aio/sqlalchemy/base.py" = [

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): CLEAN

  • Reviewer: Codex CLI 0.162.1, model gpt-6-astra, reasoning effort max, sandbox read-only, session 01a13002-267f-7c01-a92e-a8981f5b31c9.
  • Scope: git diff a667abd4a8f8d91ff6b68fd8e3b49e12fe8f0a0b..14f636c3a7858ead1201a6d5894ea01e1923bae5 on a clean detached snapshot of 14f636c, plus the locked SQLAlchemy 2.0.46 sources. The prompt left out the PR number, the description, and the self-review results.
  • Constraints: review only, so no edits, builds, tests, network, or GitHub access. This is a static review. The snapshot and the PR worktree were unchanged afterwards.

Reviewer output:

Covered the exact four-file diff and supplied patch, repository conventions, the full tracked tree, dialect registration and callers, Ruff/pytest configuration, changed tests, and the supplied SQLAlchemy 2.0.46 sources.

CLEAN — no actionable defects introduced by this diff.

  • References: No old adapter names or former test-class references remain in tracked files. Imports, annotations, docstrings, constructors, factories, and string-based registration paths are consistent. No compatibility aliases remain.
  • SQLAlchemy compatibility: SQLAlchemy constructs the DBAPI through dialect factories and calls its methods; these paths do not require the old names or inheritance from AsyncAdapt_*. The connection adapter retains AdaptedConnection inheritance. That interface predates the declared >=2.0.0 minimum.
  • Ruff: Removing N801 is correct: all classes in the module now use CapWords. No other exclusion needs changing. The compiler’s N802 exclusion remains necessary for SQLAlchemy’s visit_UPPERCASE dispatch.
  • Naming: AthenaAioAdaptedCursor, AthenaAioAdaptedConnection, and AthenaAioAdaptedDBAPI consistently follow the package’s existing AthenaAio* naming.
  • Tests: All three changed Python files exactly match their base versions after applying the intended name substitutions. Assertions, parameter IDs, fixtures, cleanup, and scenario coverage are preserved. TestAthenaAioAdaptedCursor retains pytest discovery and follows the repository’s object-focused test-class convention.

The direct-import break is intentional. This is a static review: no lint, builds, tests, or network access occurred. Dependency inspection covered 2.0.46, not every release permitted by the unbounded >=2.0.0 requirement. HEAD and the clean working tree remain unchanged.

"N801", # SQLAlchemy async adapter naming convention (AsyncAdapt_*)
]

[tool.mypy]
follow_imports = "silent"
Expand Down
8 changes: 4 additions & 4 deletions tests/pyathena/aio/sqlalchemy/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@
from sqlalchemy.sql.schema import Column, MetaData, Table
from sqlalchemy.util.concurrency import greenlet_spawn

from pyathena.aio.sqlalchemy.base import AsyncAdapt_pyathena_cursor
from pyathena.aio.sqlalchemy.base import AthenaAioAdaptedCursor
from pyathena.sqlalchemy.types import AthenaArray
from tests import ENV
from tests.pyathena.util import throttle_metadata_api


class TestAsyncAdaptPyathenaCursor:
class TestAthenaAioAdaptedCursor:
@pytest.mark.parametrize(
"error",
[ValueError("conversion failed"), asyncio.CancelledError()],
Expand All @@ -29,7 +29,7 @@ async def test_execute_closes_cursor_off_event_loop_when_fetch_fails(self, error
cursor.fetchall = AsyncMock(side_effect=error)
closes = []
cursor.close.side_effect = lambda: closes.append(threading.get_ident())
adapted = AsyncAdapt_pyathena_cursor(cursor)
adapted = AthenaAioAdaptedCursor(cursor)

with pytest.raises(type(error)):
await greenlet_spawn(adapted.execute, "SELECT 1")
Expand All @@ -48,7 +48,7 @@ async def test_execute_keeps_fetch_error_when_close_fails(self, error, caplog):
cursor.description = [("col", "varchar", None, None, None, None, None)]
cursor.fetchall = AsyncMock(side_effect=error)
cursor.close.side_effect = ValueError("generator already executing")
adapted = AsyncAdapt_pyathena_cursor(cursor)
adapted = AthenaAioAdaptedCursor(cursor)

with pytest.raises(type(error)) as raised:
await greenlet_spawn(adapted.execute, "SELECT 1")
Expand Down
4 changes: 2 additions & 2 deletions tests/pyathena/sqlalchemy/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
from pyathena.aio.connection import AioConnection
from pyathena.aio.cursor import AioCursor
from pyathena.aio.pandas.cursor import AioPandasCursor
from pyathena.aio.sqlalchemy.base import AsyncAdapt_pyathena_connection, AthenaAioDialect
from pyathena.aio.sqlalchemy.base import AthenaAioAdaptedConnection, AthenaAioDialect
from pyathena.connection import Connection
from pyathena.cursor import Cursor
from pyathena.error import DatabaseError, OperationalError
Expand Down Expand Up @@ -208,7 +208,7 @@ def test_internal_cursor_leaves_out_default_cursor_options(
cursor_kwargs={"unload": True, "chunksize": 10, "arraysize": 5000},
)
driver_connection = (
AsyncAdapt_pyathena_connection(MagicMock(), conn)
AthenaAioAdaptedConnection(MagicMock(), conn)
if connection_class is AioConnection
else conn
)
Expand Down
Loading