Repository navigation
Rename the asyncio SQLAlchemy adapter classes to CapWords and enforce N801 #1151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,7 +49,7 @@ | |
| _ASYNC_CURSOR_CLASSES: dict[Any, Any] = {Cursor: AioCursor} | ||
|
|
||
|
|
||
| class AsyncAdapt_pyathena_cursor: | ||
| class AthenaAioAdaptedCursor: | ||
| """Wraps any async PyAthena cursor with a sync DBAPI interface. | ||
|
|
||
| SQLAlchemy's async engine uses greenlet-based ``await_only()`` to call | ||
|
|
@@ -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: | ||
|
|
@@ -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 | ||
|
|
@@ -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()``. | ||
|
|
@@ -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.""" | ||
|
|
@@ -319,7 +319,7 @@ def rollback(self) -> None: | |
| """Do nothing, because Athena does not support transactions.""" | ||
|
|
||
|
|
||
| class AsyncAdapt_pyathena_dbapi: | ||
| class AthenaAioAdaptedDBAPI: | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Self-review round two (compatibility, operations, factual claims): CLEAN Scope:
Findings: none.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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), |
||
| """Fake DBAPI module for the async SQLAlchemy engine. | ||
|
|
||
| SQLAlchemy expects ``import_dbapi()`` to return a module-like object | ||
|
|
@@ -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): | ||
|
|
@@ -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]]: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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" = [ | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Independent review (relayed): CLEAN
Reviewer output:
|
||
| "N801", # SQLAlchemy async adapter naming convention (AsyncAdapt_*) | ||
| ] | ||
|
|
||
| [tool.mypy] | ||
| follow_imports = "silent" | ||
|
|
||
There was a problem hiding this comment.
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).__slots__, and theimport_dbapi()/dbapi()return objects are unchanged. SQLAlchemy receives the DBAPI object fromimport_dbapi()and never looks the classes up by name;git grep -i asyncadaptfinds no string-based or dynamic lookup,__all__, or Sphinxautoclassentry for them (only SQLAlchemy's ownpool.AsyncAdaptedQueuePoolremains).awsathena+aio*entry points inpyproject.toml:68-72name theAthenaAio*Dialectclasses, not the adapters.just lintgreen, andruff check --select N801rejects a temporaryclass AsyncAdapt_pyathena_cursor:underpyathena/aio/sqlalchemy/. The N802 exclusion forcompiler.pystays as required.TestAthenaAioAdaptedCursor) follow the rename; no new behavior needs new coverage.tests/pyathena/aio/sqlalchemy/(53 passed) andjust test sqla-async(589 passed, 759 skipped) exercise the adapters end to end.Findings: none.