From 46c394cb27968b4f981df903f15b5d6f41caf538 Mon Sep 17 00:00:00 2001 From: karezche <64801825+karenc-bq@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:17:38 -0700 Subject: [PATCH 1/4] fix(sqlalchemy): let failover errors reach SQLAlchemy unchanged --- aws_advanced_python_wrapper/errors.py | 12 +- aws_advanced_python_wrapper/pep249.py | 2 + .../_exception_handling.py | 134 ++++++++---------- .../sqlalchemy_dialects/mysql.py | 23 ++- .../sqlalchemy_dialects/mysql_async.py | 15 +- .../sqlalchemy_dialects/pg.py | 17 +-- .../sqlalchemy_dialects/pg_async.py | 14 +- tests/unit/test_aio_sqlalchemy_dialect.py | 53 ++++--- .../test_failover_success_error_isolation.py | 15 +- 9 files changed, 125 insertions(+), 160 deletions(-) diff --git a/aws_advanced_python_wrapper/errors.py b/aws_advanced_python_wrapper/errors.py index fa250828c..7af9a752e 100644 --- a/aws_advanced_python_wrapper/errors.py +++ b/aws_advanced_python_wrapper/errors.py @@ -51,11 +51,13 @@ class FailoverFailedError(FailoverError): class FailoverSuccessError(FailoverError): - # SA classification is handled at the dialect boundary by - # ``sqlalchemy_dialects._exception_handling._FailoverSuccessRewrapMixin``, - # which catches FailoverSuccessError in ``do_execute`` / - # ``do_executemany`` and re-raises as the dialect's native - # OperationalError. Do NOT add driver-native OperationalError classes + # SA classification needs no help at the dialect boundary: ``FailoverError`` + # derives from the wrapper's own ``pep249.OperationalError``, and SA's + # ``DBAPIError.instance`` matches ``orig.__class__.__mro__`` by class *name*, + # so this already maps to ``sqlalchemy.exc.OperationalError`` with + # ``DBAPIError.orig`` left as this exception. + # + # Do NOT add driver-native OperationalError classes # (psycopg / mysql.connector / aiomysql) as bases here: Django's # ``wrap_database_errors`` walks ``issubclass`` against the driver's # own error module and would swallow FailoverSuccessError before any diff --git a/aws_advanced_python_wrapper/pep249.py b/aws_advanced_python_wrapper/pep249.py index f09aa523d..3e486b17f 100644 --- a/aws_advanced_python_wrapper/pep249.py +++ b/aws_advanced_python_wrapper/pep249.py @@ -47,6 +47,8 @@ class Warning(Exception): class Error(Exception): __module__ = "aws_advanced_python_wrapper" + errno = None + sqlstate = None class InterfaceError(Error): diff --git a/aws_advanced_python_wrapper/sqlalchemy_dialects/_exception_handling.py b/aws_advanced_python_wrapper/sqlalchemy_dialects/_exception_handling.py index af48c7465..ef7a41321 100644 --- a/aws_advanced_python_wrapper/sqlalchemy_dialects/_exception_handling.py +++ b/aws_advanced_python_wrapper/sqlalchemy_dialects/_exception_handling.py @@ -14,40 +14,58 @@ """Shared exception-handling helpers for the wrapper's SA dialects. -SQLAlchemy classifies DBAPI exceptions in ``Connection._handle_dbapi_exception`` -by walking ``dialect.loaded_dbapi.`` and wrapping into -``sqlalchemy.exc.``. SA's classifier needs the raised exception -to be an instance of the **driver-native** ``OperationalError`` class -(e.g. ``psycopg.OperationalError``), not the wrapper's PEP-249 -``OperationalError``. Wrapper-internal exceptions like -``FailoverSuccessError`` are single-inherit from ``FailoverError`` (the -driver-native multi-inheritance was reverted in commit ``d994d02`` because -it caused Django's ``wrap_database_errors`` to swallow the failover signal -on MySQL), so SA's classifier lets them escape raw and any user-written -``except sqlalchemy.exc.OperationalError:`` retry loop never fires. - -The mixin below sidesteps that by intercepting ``FailoverSuccessError`` at -the ``do_execute`` / ``do_executemany`` boundary and re-raising it as the -driver-native ``OperationalError`` class — which SA's classifier DOES -reclassify reliably to ``sqlalchemy.exc.OperationalError``. The original -wrapper exception is preserved via ``__cause__`` so callers that need the -exact wrapper type can ``isinstance(exc.__cause__, FailoverSuccessError)``. - -Each concrete dialect declares its target class via -``_failover_success_target_cls``; the mixin handles both sync and async -``do_execute`` shapes. - -Scope: only ``do_execute`` and ``do_executemany`` are wrapped. If -``FailoverSuccessError`` ever surfaces from ``do_commit`` / ``do_rollback`` -/ ``do_begin_twophase`` / etc., it will escape raw — extend the mixin then. +How SQLAlchemy classifies the wrapper's errors +---------------------------------------------- +``sqlalchemy.exc.DBAPIError.instance`` walks ``orig.__class__.__mro__`` and +matches each base **by class name** against the names exported from +``sqlalchemy.exc``, gated on ``isinstance(orig, dialect.loaded_dbapi.Error)``. +Our ``loaded_dbapi`` is the wrapper itself, so that gate is the wrapper's +PEP-249 ``Error`` — and because ``FailoverError`` derives from the wrapper's +``pep249.OperationalError``, every failover error already carries a base *named* +``OperationalError`` in its MRO. SA therefore maps the whole family to +``sqlalchemy.exc.OperationalError`` on its own. The class does NOT need to be +driver-native, and nothing at the dialect boundary needs to re-raise anything +for ``except sqlalchemy.exc.OperationalError:`` to fire. + +The wrapper's own errors are therefore passed through ``do_execute`` UNCHANGED. +Two things depend on that: + +* SA puts the original exception in ``DBAPIError.orig``, so a consumer can write + ``isinstance(err.orig, FailoverSuccessError)`` — the documented idiom — as + well as ``except sqlalchemy.exc.OperationalError:``. +* Each dialect's ``is_disconnect`` override sees the real ``FailoverError`` and + decides pool invalidation from it: ``FailoverFailedError`` -> invalidate, + ``FailoverSuccessError`` / ``TransactionResolutionUnknownError`` -> keep the + pooled connection, which the wrapper has already rebound to the new writer. + +**Do not reintroduce a rewrap here.** An earlier version of this module caught +``FailoverSuccessError`` and re-raised ``pep249.OperationalError``. It gained +nothing — SA produced the same ``sqlalchemy.exc.OperationalError`` either way — +and it cost two things: + +1. ``DBAPIError.orig`` became the substitute, silently breaking + ``isinstance(err.orig, FailoverSuccessError)``. +2. The substitute is not a ``FailoverError``, so ``is_disconnect`` below no + longer recognised it and fell through to + ``MySQLDialect_mysqlconnector.is_disconnect``, which probes ``e.errno``. The + wrapper's PEP-249 errors carry no ``errno``, and SA calls ``is_disconnect`` + at the top of ``Connection._handle_dbapi_exception`` with no enclosing + ``try:`` — so an ``AttributeError`` escaped and the consumer's + ``except DBAPIError:`` never ran at all. Confirmed on a real Aurora MySQL + failover: a successful failover surfaced to the application as + ``AttributeError: 'OperationalError' object has no attribute 'errno'``. + +What this module DOES do +------------------------ +Normalize *raw driver-native* DBAPI errors (``mysql.connector.errors.*`` / +``psycopg.*`` / ``pymysql.*``) into the wrapper's PEP-249 equivalents, for plugin +chains that do not already re-wrap them (``iam`` / ``aws_secrets_manager`` / no +plugins). Without that SA cannot classify them at all, so e.g. ``has_table`` +never sees MySQL's 1146 and ``create_all`` fails. """ from __future__ import annotations -from typing import ClassVar, Optional, Type - -from aws_advanced_python_wrapper.errors import FailoverSuccessError - def _normalize_driver_error(e, driver_error_module): """Translate a raw driver-native DBAPI error into the wrapper's PEP-249 @@ -98,27 +116,15 @@ def _normalize_driver_error(e, driver_error_module): return wrapped -class _FailoverSuccessRewrapMixin: - """Re-raise ``FailoverSuccessError`` as the driver-native OperationalError. - - Concrete dialect subclasses set ``_failover_success_target_cls`` to the - driver's own ``OperationalError`` class (e.g. ``psycopg.OperationalError``, - ``mysql.connector.errors.OperationalError``, ``aiomysql.OperationalError``). - The mixin's ``do_execute`` wraps the parent's call: on - ``FailoverSuccessError``, it raises the target class with the same message, - chaining the original via ``__cause__``. SA's classifier reliably maps - driver-native ``OperationalError`` -> ``sqlalchemy.exc.OperationalError``, - so user retry loops (``except sqlalchemy.exc.OperationalError:``) fire. +class _DriverErrorNormalizeMixin: + """Normalize raw driver-native DBAPI errors into the wrapper's PEP-249 types. - Other ``FailoverError`` subclasses (``FailoverFailedError``, - ``TransactionResolutionUnknownError``) are NOT rewrapped: failed failover - is a hard error the user should see, and transaction-resolution-unknown - has its own semantics distinct from a generic OperationalError. + The wrapper's own errors — including the whole ``FailoverError`` family — are + passed through UNTOUCHED, so ``DBAPIError.orig`` stays the exception the + wrapper raised and each dialect's ``is_disconnect`` can classify it. See the + module docstring for why re-raising a substitute class here is wrong. """ - # Subclasses MUST set this to the driver-native OperationalError class. - _failover_success_target_cls: ClassVar[Optional[Type[BaseException]]] = None - def _driver_error_module(self): """Driver-native DBAPI exception namespace (module exposing PEP-249 error classes: ``Error``, ``OperationalError``, ``ProgrammingError``, @@ -134,11 +140,6 @@ def do_execute( # type: ignore[no-untyped-def] try: super().do_execute( # type: ignore[misc] cursor, statement, parameters, context) - except FailoverSuccessError as e: - target = self._failover_success_target_cls - if target is None: - raise # mis-configured dialect; surface the raw error - raise target(str(e)) from e except Exception as e: normalized = _normalize_driver_error(e, self._driver_error_module()) if normalized is not None: @@ -150,11 +151,6 @@ def do_executemany( # type: ignore[no-untyped-def] try: super().do_executemany( # type: ignore[misc] cursor, statement, parameters, context) - except FailoverSuccessError as e: - target = self._failover_success_target_cls - if target is None: - raise - raise target(str(e)) from e except Exception as e: normalized = _normalize_driver_error(e, self._driver_error_module()) if normalized is not None: @@ -162,8 +158,8 @@ def do_executemany( # type: ignore[no-untyped-def] raise -class _AsyncFailoverSuccessRewrapMixin: - """Async counterpart of :class:`_FailoverSuccessRewrapMixin`. +class _AsyncDriverErrorNormalizeMixin: + """Async counterpart of :class:`_DriverErrorNormalizeMixin`. IMPORTANT: ``do_execute`` / ``do_executemany`` MUST be SYNCHRONOUS even for async dialects. SQLAlchemy's execution context calls @@ -176,13 +172,11 @@ class _AsyncFailoverSuccessRewrapMixin: ``dialect.initialize``'s ``SELECT version()`` (the ``sqlalchemy_creator_*`` integration tests). So this mixin is functionally identical to the sync one; it exists as a distinct class only so async dialects can be wired to a - different ``_failover_success_target_cls`` / ``_driver_error_module``. + different ``_driver_error_module``. """ - _failover_success_target_cls: ClassVar[Optional[Type[BaseException]]] = None - def _driver_error_module(self): - """See :meth:`_FailoverSuccessRewrapMixin._driver_error_module`.""" + """See :meth:`_DriverErrorNormalizeMixin._driver_error_module`.""" return None def do_execute( # type: ignore[no-untyped-def] @@ -190,11 +184,6 @@ def do_execute( # type: ignore[no-untyped-def] try: super().do_execute( # type: ignore[misc] cursor, statement, parameters, context) - except FailoverSuccessError as e: - target = self._failover_success_target_cls - if target is None: - raise - raise target(str(e)) from e except Exception as e: normalized = _normalize_driver_error(e, self._driver_error_module()) if normalized is not None: @@ -206,11 +195,6 @@ def do_executemany( # type: ignore[no-untyped-def] try: super().do_executemany( # type: ignore[misc] cursor, statement, parameters, context) - except FailoverSuccessError as e: - target = self._failover_success_target_cls - if target is None: - raise - raise target(str(e)) from e except Exception as e: normalized = _normalize_driver_error(e, self._driver_error_module()) if normalized is not None: @@ -218,4 +202,4 @@ def do_executemany( # type: ignore[no-untyped-def] raise -__all__ = ["_FailoverSuccessRewrapMixin", "_AsyncFailoverSuccessRewrapMixin"] +__all__ = ["_DriverErrorNormalizeMixin", "_AsyncDriverErrorNormalizeMixin"] diff --git a/aws_advanced_python_wrapper/sqlalchemy_dialects/mysql.py b/aws_advanced_python_wrapper/sqlalchemy_dialects/mysql.py index 200451ccd..6a7ddbdf9 100644 --- a/aws_advanced_python_wrapper/sqlalchemy_dialects/mysql.py +++ b/aws_advanced_python_wrapper/sqlalchemy_dialects/mysql.py @@ -26,26 +26,17 @@ from sqlalchemy.dialects.mysql.mysqlconnector import \ MySQLDialect_mysqlconnector -from aws_advanced_python_wrapper.pep249 import \ - OperationalError as _PEP249OperationalError from aws_advanced_python_wrapper.sqlalchemy_dialects._exception_handling import \ - _FailoverSuccessRewrapMixin + _DriverErrorNormalizeMixin class AwsWrapperMySQLConnectorDialect( - _FailoverSuccessRewrapMixin, MySQLDialect_mysqlconnector): + _DriverErrorNormalizeMixin, MySQLDialect_mysqlconnector): """SQLAlchemy dialect that uses the AWS Advanced Python Wrapper as its DBAPI.""" driver = "aws_wrapper_mysqlconnector" supports_statement_cache = True - # See _FailoverSuccessRewrapMixin / sqlalchemy_dialects/pg.py for the - # full rationale. The shim's ``dialect.dbapi.OperationalError`` resolves - # to the wrapper's PEP-249 ``OperationalError`` via ``_dbapi.install``, - # so the rewrap target must be that class for SA's classifier to wrap - # us to ``sqlalchemy.exc.OperationalError``. - _failover_success_target_cls = _PEP249OperationalError - @classmethod def import_dbapi(cls): import aws_advanced_python_wrapper.mysql_connector as dbapi @@ -113,9 +104,13 @@ def is_disconnect(self, e, connection, cursor): # psycopg's upstream is_disconnect returns False for this.) # - FailoverFailedError: the wrapper has no working connection; # pool slot really is dead. Return True so SA invalidates. - # _FailoverSuccessRewrapMixin still handles the do_execute path; - # this method handles the cursor-creation path which runs before - # do_execute reaches the mixin. + # This is the ONLY handler for the whole FailoverError family, on both + # paths: cursor creation and do_execute. Nothing at the do_execute + # boundary substitutes the exception any more -- an earlier rewrap there + # made this method unreachable for FailoverSuccessError, which then fell + # through to upstream's e.errno probe and raised AttributeError out of + # SA's unguarded is_disconnect call. See + # sqlalchemy_dialects/_exception_handling.py. from aws_advanced_python_wrapper.errors import (FailoverError, FailoverFailedError) diff --git a/aws_advanced_python_wrapper/sqlalchemy_dialects/mysql_async.py b/aws_advanced_python_wrapper/sqlalchemy_dialects/mysql_async.py index 7065f4a4e..58d056caf 100644 --- a/aws_advanced_python_wrapper/sqlalchemy_dialects/mysql_async.py +++ b/aws_advanced_python_wrapper/sqlalchemy_dialects/mysql_async.py @@ -44,10 +44,8 @@ from sqlalchemy.engine.characteristics import ConnectionCharacteristic from sqlalchemy.util.concurrency import await_only -from aws_advanced_python_wrapper.pep249 import \ - OperationalError as _PEP249OperationalError from aws_advanced_python_wrapper.sqlalchemy_dialects._exception_handling import \ - _AsyncFailoverSuccessRewrapMixin + _AsyncDriverErrorNormalizeMixin def _unwrap_wrapper_conn(dbapi_conn: Any) -> Any: @@ -130,7 +128,7 @@ def connect(self, *args: Any, **kwargs: Any) -> AsyncAdapt_aiomysql_connection: class AwsWrapperMySQLAiomysqlAsyncDialect( - _AsyncFailoverSuccessRewrapMixin, MySQLDialect_aiomysql): + _AsyncDriverErrorNormalizeMixin, MySQLDialect_aiomysql): """Async SQLAlchemy dialect that uses the AWS Advanced Python Wrapper as its DBAPI.""" driver = "aws_wrapper_aiomysql" @@ -146,13 +144,6 @@ class AwsWrapperMySQLAiomysqlAsyncDialect( "mysql_readonly": _MySQLReadOnlyConnectionCharacteristic(), }) - # See _AsyncFailoverSuccessRewrapMixin / sqlalchemy_dialects/pg.py. - # ``dialect.dbapi.OperationalError`` resolves to the wrapper's PEP-249 - # ``OperationalError`` via the shim's ``_dbapi.install`` — rewrap - # target must be that class for SA's classifier to wrap us to - # ``sqlalchemy.exc.OperationalError``. - _failover_success_target_cls = _PEP249OperationalError - def set_readonly(self, dbapi_conn: Any, value: bool) -> None: # dbapi_conn is SA's AsyncAdapt_aiomysql_connection; the wrapper # connection (whose set_read_only the RWS plugin intercepts) is at @@ -238,7 +229,7 @@ def is_disconnect(self, e, connection, cursor): # which is now demoted to a reader. # - FailoverFailedError → wrapper has no working connection; # return True so SA invalidates and the creator retries. - # _AsyncFailoverSuccessRewrapMixin handles do_execute path; + # _AsyncDriverErrorNormalizeMixin handles the do_execute path; # this handles the cursor-creation path that runs earlier. from aws_advanced_python_wrapper.errors import (FailoverError, FailoverFailedError) diff --git a/aws_advanced_python_wrapper/sqlalchemy_dialects/pg.py b/aws_advanced_python_wrapper/sqlalchemy_dialects/pg.py index b95c33593..a5a9728ca 100644 --- a/aws_advanced_python_wrapper/sqlalchemy_dialects/pg.py +++ b/aws_advanced_python_wrapper/sqlalchemy_dialects/pg.py @@ -29,13 +29,11 @@ from sqlalchemy.dialects.postgresql.psycopg import PGDialect_psycopg -from aws_advanced_python_wrapper.pep249 import \ - OperationalError as _PEP249OperationalError from aws_advanced_python_wrapper.sqlalchemy_dialects._exception_handling import \ - _FailoverSuccessRewrapMixin + _DriverErrorNormalizeMixin -class AwsWrapperPGPsycopgDialect(_FailoverSuccessRewrapMixin, PGDialect_psycopg): +class AwsWrapperPGPsycopgDialect(_DriverErrorNormalizeMixin, PGDialect_psycopg): """SQLAlchemy dialect that uses the AWS Advanced Python Wrapper as its DBAPI. Wrapper-specific override pattern @@ -56,17 +54,6 @@ class AwsWrapperPGPsycopgDialect(_FailoverSuccessRewrapMixin, PGDialect_psycopg) driver = "aws_wrapper_psycopg" supports_statement_cache = True - # See _FailoverSuccessRewrapMixin. SA's classifier checks - # ``isinstance(exc, dialect.dbapi.OperationalError)``; for our shim - # ``dialect.dbapi.OperationalError`` resolves to the wrapper's PEP-249 - # ``OperationalError`` (installed via ``_dbapi.install``), NOT psycopg's - # native one. The target class must therefore be the wrapper's PEP-249 - # class so SA's classifier matches and wraps to - # ``sqlalchemy.exc.OperationalError``. (psycopg.OperationalError would - # only work if SA's dbapi attribute pointed at the real psycopg module, - # which it doesn't here because ``import_dbapi`` returns our shim.) - _failover_success_target_cls = _PEP249OperationalError - @classmethod def import_dbapi(cls): import aws_advanced_python_wrapper.psycopg as dbapi diff --git a/aws_advanced_python_wrapper/sqlalchemy_dialects/pg_async.py b/aws_advanced_python_wrapper/sqlalchemy_dialects/pg_async.py index ac84dd80c..132640d76 100644 --- a/aws_advanced_python_wrapper/sqlalchemy_dialects/pg_async.py +++ b/aws_advanced_python_wrapper/sqlalchemy_dialects/pg_async.py @@ -44,10 +44,8 @@ from sqlalchemy.util import asbool from sqlalchemy.util.concurrency import await_fallback, await_only -from aws_advanced_python_wrapper.pep249 import \ - OperationalError as _PEP249OperationalError from aws_advanced_python_wrapper.sqlalchemy_dialects._exception_handling import \ - _AsyncFailoverSuccessRewrapMixin + _AsyncDriverErrorNormalizeMixin class AwsWrapperAsyncPsycopgAdaptDBAPI: @@ -110,7 +108,7 @@ def connect(self, *args: Any, **kwargs: Any) -> AsyncAdapt_psycopg_connection: class AwsWrapperPGPsycopgAsyncDialect( - _AsyncFailoverSuccessRewrapMixin, PGDialectAsync_psycopg): + _AsyncDriverErrorNormalizeMixin, PGDialectAsync_psycopg): """Async SQLAlchemy dialect that uses the AWS Advanced Python Wrapper as its DBAPI. Wrapper-specific override pattern @@ -138,12 +136,6 @@ class AwsWrapperPGPsycopgAsyncDialect( driver = "aws_wrapper_psycopg" supports_statement_cache = True - # See _AsyncFailoverSuccessRewrapMixin / sqlalchemy_dialects/pg.py. - # ``dialect.dbapi.OperationalError`` resolves to the wrapper's PEP-249 - # ``OperationalError`` via the shim's ``_dbapi.install`` — rewrap - # target must be that class for SA's classifier to wrap us to - # ``sqlalchemy.exc.OperationalError``. - _failover_success_target_cls = _PEP249OperationalError is_async = True def _driver_error_module(self): @@ -160,7 +152,7 @@ def is_disconnect(self, e, connection, cursor): # auto-rebound to the new writer via plugin_service; SA pool # slot is still valid). # - FailoverFailedError → True (no usable connection). - # Complements _AsyncFailoverSuccessRewrapMixin for the + # Complements _AsyncDriverErrorNormalizeMixin for the # cursor-creation path that runs before do_execute. from aws_advanced_python_wrapper.errors import (FailoverError, FailoverFailedError) diff --git a/tests/unit/test_aio_sqlalchemy_dialect.py b/tests/unit/test_aio_sqlalchemy_dialect.py index 97de66936..28a1c5a4a 100644 --- a/tests/unit/test_aio_sqlalchemy_dialect.py +++ b/tests/unit/test_aio_sqlalchemy_dialect.py @@ -260,22 +260,21 @@ def test_async_failover_rewrap_do_execute_is_synchronous(): import inspect from aws_advanced_python_wrapper.sqlalchemy_dialects._exception_handling import \ - _AsyncFailoverSuccessRewrapMixin + _AsyncDriverErrorNormalizeMixin assert not inspect.iscoroutinefunction( - _AsyncFailoverSuccessRewrapMixin.do_execute) + _AsyncDriverErrorNormalizeMixin.do_execute) assert not inspect.iscoroutinefunction( - _AsyncFailoverSuccessRewrapMixin.do_executemany) + _AsyncDriverErrorNormalizeMixin.do_executemany) -def test_async_failover_rewrap_runs_parent_and_rewraps_failover_success(): +def test_async_mixin_runs_parent_and_passes_failover_errors_through(): import pytest - from aws_advanced_python_wrapper.errors import FailoverSuccessError + from aws_advanced_python_wrapper.errors import ( + FailoverFailedError, FailoverSuccessError, + TransactionResolutionUnknownError) from aws_advanced_python_wrapper.sqlalchemy_dialects._exception_handling import \ - _AsyncFailoverSuccessRewrapMixin - - class _Target(Exception): - pass + _AsyncDriverErrorNormalizeMixin calls = [] @@ -283,23 +282,37 @@ class _Parent: def do_execute(self, cursor, statement, parameters, context=None): calls.append((statement, parameters)) - class _Dialect(_AsyncFailoverSuccessRewrapMixin, _Parent): - _failover_success_target_cls = _Target + class _Dialect(_AsyncDriverErrorNormalizeMixin, _Parent): + pass # Synchronous call actually invokes the parent => the query runs. _Dialect().do_execute(MagicMock(), "select 1", None) assert calls == [("select 1", None)] - class _ParentRaises: - def do_execute(self, *a, **k): - raise FailoverSuccessError("failover") - - class _DialectRaises(_AsyncFailoverSuccessRewrapMixin, _ParentRaises): - _failover_success_target_cls = _Target + # Regression guard. The wrapper's failover errors MUST reach SQLAlchemy + # unchanged. An earlier version of this mixin re-raised FailoverSuccessError + # as pep249.OperationalError, which (a) replaced DBAPIError.orig, breaking + # the documented ``isinstance(err.orig, FailoverSuccessError)`` idiom, and + # (b) hid the error from the dialects' is_disconnect override, so upstream's + # ``e.errno`` probe raised AttributeError from a call site SQLAlchemy does + # not guard -- the consumer's ``except DBAPIError`` never ran. + for err_cls in (FailoverSuccessError, TransactionResolutionUnknownError, + FailoverFailedError): + raised = err_cls("failover") + + class _ParentRaises: + def do_execute(self, *a, **k): + raise raised + + class _DialectRaises(_AsyncDriverErrorNormalizeMixin, _ParentRaises): + pass - # FailoverSuccessError from the driver is rewrapped to the target class. - with pytest.raises(_Target): - _DialectRaises().do_execute(MagicMock(), "select 1", None) + with pytest.raises(err_cls) as exc_info: + _DialectRaises().do_execute(MagicMock(), "select 1", None) + assert exc_info.value is raised, ( + f"do_execute replaced {err_cls.__name__} with " + f"{type(exc_info.value).__name__}; DBAPIError.orig would no longer " + f"be the wrapper's own error") def test_async_dialect_type_info_fetch_falls_through_without_wrapper(mocker): diff --git a/tests/unit/test_failover_success_error_isolation.py b/tests/unit/test_failover_success_error_isolation.py index b27c48341..a8bb74c2f 100644 --- a/tests/unit/test_failover_success_error_isolation.py +++ b/tests/unit/test_failover_success_error_isolation.py @@ -18,14 +18,13 @@ PostgreSQL if/when a Django-PG backend ships in the wrapper. SA classification of FailoverSuccessError to ``sqlalchemy.exc.OperationalError`` -happens at the dialect boundary via -``aws_advanced_python_wrapper.sqlalchemy_dialects._exception_handling -._FailoverSuccessRewrapMixin``, which catches FailoverSuccessError in -``do_execute`` / ``do_executemany`` and re-raises as the dialect's native -``OperationalError``. That mechanism does NOT require the driver-native -multi-inheritance below; if someone re-introduces it, Django (and any -other consumer that walks ``issubclass`` against driver error modules) -will start wrapping failover signals. +comes from ``errors.py``, not from the dialects: ``FailoverError`` derives from +the wrapper's ``pep249.OperationalError``, and SA's ``DBAPIError.instance`` +matches ``orig.__class__.__mro__`` by class *name*, so a base named +``OperationalError`` is all it needs. That mechanism does NOT require the +driver-native multi-inheritance below; if someone re-introduces it, Django (and +any other consumer that walks ``issubclass`` against driver error modules) +will start swallowing failover signals. Regression these tests guard against: see tests/integration/container/django/test_django_plugins.py:: From a01b062b86ffb6cf320e675f9bcfda3775702d1d Mon Sep 17 00:00:00 2001 From: karezche <64801825+karenc-bq@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:17:48 -0700 Subject: [PATCH 2/4] fix: drop type stubs from the runtime dependencies --- poetry.lock | 14 +++++++------- pyproject.toml | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/poetry.lock b/poetry.lock index eb15cff7f..5abd594f6 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 2.4.1 and should not be changed by hand. +# This file is automatically @generated by Poetry 2.3.3 and should not be changed by hand. [[package]] name = "aiohappyeyeballs" @@ -297,7 +297,7 @@ version = "1.43.32" description = "Type annotations for boto3 1.43.32 generated with mypy-boto3-builder 8.12.0" optional = false python-versions = ">=3.9" -groups = ["main"] +groups = ["dev"] files = [ {file = "boto3_stubs-1.43.32-py3-none-any.whl", hash = "sha256:7d4528dba959c9e2158904c857c0aeb54b0dcdfd95d2fc54a8e97b916cfe04a8"}, {file = "boto3_stubs-1.43.32.tar.gz", hash = "sha256:d63a679b19c124226988b1dba269cb58280175ac7435673212c10904fcffaee8"}, @@ -765,7 +765,7 @@ version = "1.38.30" description = "Type annotations and code completion for botocore" optional = false python-versions = ">=3.8" -groups = ["main", "test"] +groups = ["dev", "test"] files = [ {file = "botocore_stubs-1.38.30-py3-none-any.whl", hash = "sha256:2efb8bdf36504aff596c670d875d8f7dd15205277c15c4cea54afdba8200c266"}, {file = "botocore_stubs-1.38.30.tar.gz", hash = "sha256:291d7bf39a316c00a8a55b7255489b02c0cea1a343482e7784e8d1e235bae995"}, @@ -2993,7 +2993,7 @@ version = "2.15.0.20251206" description = "Typing stubs for aws-xray-sdk" optional = false python-versions = ">=3.9" -groups = ["main"] +groups = ["dev"] files = [ {file = "types_aws_xray_sdk-2.15.0.20251206-py3-none-any.whl", hash = "sha256:1c73b3737027a5607450e6e0313c54947801faa2be3906b10d75602b94cee7f8"}, {file = "types_aws_xray_sdk-2.15.0.20251206.tar.gz", hash = "sha256:c4283920a0933f0452f0fc01e9f047b5e385767d66a70dc6ed5fad34dc91b803"}, @@ -3005,7 +3005,7 @@ version = "0.27.2" description = "Type annotations and code completion for awscrt" optional = false python-versions = ">=3.8" -groups = ["main", "test"] +groups = ["dev", "test"] files = [ {file = "types_awscrt-0.27.2-py3-none-any.whl", hash = "sha256:49a045f25bbd5ad2865f314512afced933aed35ddbafc252e2268efa8a787e4e"}, {file = "types_awscrt-0.27.2.tar.gz", hash = "sha256:acd04f57119eb15626ab0ba9157fc24672421de56e7bd7b9f61681fedee44e91"}, @@ -3477,7 +3477,7 @@ version = "0.13.0" description = "Type annotations and code completion for s3transfer" optional = false python-versions = ">=3.8" -groups = ["main", "test"] +groups = ["dev", "test"] files = [ {file = "types_s3transfer-0.13.0-py3-none-any.whl", hash = "sha256:79c8375cbf48a64bff7654c02df1ec4b20d74f8c5672fc13e382f593ca5565b3"}, {file = "types_s3transfer-0.13.0.tar.gz", hash = "sha256:203dadcb9865c2f68fb44bc0440e1dc05b79197ba4a641c0976c26c9af75ef52"}, @@ -3728,4 +3728,4 @@ propcache = ">=0.2.1" [metadata] lock-version = "2.1" python-versions = "^3.10.0" -content-hash = "d7f2964091ea48399d32d486b3c3295aad748c7e9aa1cfe9652d4eab9aa9c882" +content-hash = "9b2be02dafe3a2a0932818825ea80f946bc602cdbba06b6d343a63830eb83c8c" diff --git a/pyproject.toml b/pyproject.toml index 0a9e50a15..e95c614bd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,11 +43,9 @@ resourcebundle = "2.1.0" boto3 = "^1.42.95" toml = "^0.10.2" aws-xray-sdk = "^2.15.0" -types_aws_xray_sdk = "^2.13.0" opentelemetry-api = "^1.22.0" opentelemetry-sdk = "^1.22.0" requests = "^2.33.1" -boto3-stubs = ">=1.43.0,<1.44.0" [tool.poetry.group.dev.dependencies] mypy = "^1.20.2" @@ -63,6 +61,8 @@ django = "^5.2.13" django-stubs = "^5.2.9" aiomysql = ">=0.2" aiohttp = ">=3" +boto3-stubs = "^1.43.0" +types_aws_xray_sdk = "^2.13.0" [tool.poetry.group.test.dependencies] boto3 = "^1.42.95" From bedffbf3f38af782542723a9f61eb0547cdbd553 Mon Sep 17 00:00:00 2001 From: karezche <64801825+karenc-bq@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:17:58 -0700 Subject: [PATCH 3/4] fix: restore the pre-3.1.0 SQLAlchemy dialect import path --- .../sqlalchemy/__init__.py | 28 +++++++++ .../sqlalchemy/mysql_orm_dialect.py | 62 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 aws_advanced_python_wrapper/sqlalchemy/__init__.py create mode 100644 aws_advanced_python_wrapper/sqlalchemy/mysql_orm_dialect.py diff --git a/aws_advanced_python_wrapper/sqlalchemy/__init__.py b/aws_advanced_python_wrapper/sqlalchemy/__init__.py new file mode 100644 index 000000000..7daa12dc1 --- /dev/null +++ b/aws_advanced_python_wrapper/sqlalchemy/__init__.py @@ -0,0 +1,28 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). +# You may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Deprecated location of the SQLAlchemy dialects. + +The dialects moved to :mod:`aws_advanced_python_wrapper.sqlalchemy_dialects` in +3.1.0. This package exists only so that ``import`` statements written against +3.0.0 keep working; it will be removed in the next major version. + +Only the MySQL name is aliased. 3.0.0's ``entry_points.txt`` also declared +``postgresql.aws_wrapper_psycopg = +aws_advanced_python_wrapper.sqlalchemy.pg_orm_dialect:SqlAlchemyOrmPgDialect``, +but that module was never present in the 3.0.0 distribution — the entry point +was dangling and ``create_engine("postgresql+aws_wrapper_psycopg://")`` raised +``ModuleNotFoundError`` (aws/aws-advanced-python-wrapper#1260, #1273). There is +therefore no working 3.0.0 PostgreSQL name to preserve. +""" diff --git a/aws_advanced_python_wrapper/sqlalchemy/mysql_orm_dialect.py b/aws_advanced_python_wrapper/sqlalchemy/mysql_orm_dialect.py new file mode 100644 index 000000000..bcdd6f605 --- /dev/null +++ b/aws_advanced_python_wrapper/sqlalchemy/mysql_orm_dialect.py @@ -0,0 +1,62 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). +# You may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Deprecated alias for the pre-3.1.0 MySQL SQLAlchemy dialect module path. + +``SqlAlchemyOrmMysqlDialect`` was renamed to +:class:`~aws_advanced_python_wrapper.sqlalchemy_dialects.mysql.AwsWrapperMySQLConnectorDialect` +and moved to :mod:`aws_advanced_python_wrapper.sqlalchemy_dialects` in 3.1.0. + +URL-based configuration was never affected: the registered driver name +``aws_wrapper_mysqlconnector`` is unchanged, so +``mysql+aws_wrapper_mysqlconnector://`` resolves in both releases. This shim +exists for the narrower case of code that imported the dialect class directly — +to subclass it, or to register it by hand — which would otherwise break with +``ModuleNotFoundError`` on upgrade. + +One behavioural difference is worth knowing, though it is not a loss of +capability. 3.0.0's ``create_connect_args`` hard-coded +``plugins = "aurora_connection_tracker,failover_v2"`` whenever ``wrapper_plugins`` +was absent from the URL. The replacement class does not set ``plugins`` at all, +so the wrapper's own default chain applies instead. For mysql-connector that +default is ``initial_connection,aurora_connection_tracker,failover_v2`` — a +superset of what 3.0.0 injected — so failover remains enabled by default. An +application that relied on the old value *exactly* (for instance to keep +``initial_connection`` out of the chain) should now pass ``wrapper_plugins`` in +the URL, or ``plugins`` via ``connect_args``, explicitly. +""" + +import warnings + +from aws_advanced_python_wrapper.sqlalchemy_dialects.mysql import \ + AwsWrapperMySQLConnectorDialect + +warnings.warn( + "aws_advanced_python_wrapper.sqlalchemy.mysql_orm_dialect." + "SqlAlchemyOrmMysqlDialect is deprecated and will be removed in the next " + "major version. Use aws_advanced_python_wrapper.sqlalchemy_dialects.mysql." + "AwsWrapperMySQLConnectorDialect instead. URL-based configuration needs no " + "change: mysql+aws_wrapper_mysqlconnector:// is unchanged. Note that the " + "replacement no longer injects plugins=aurora_connection_tracker," + "failover_v2; the wrapper's default chain applies instead, which for " + "mysql-connector is a superset of it.", + DeprecationWarning, + stacklevel=2, +) + +#: Deprecated alias. Kept so 3.0.0 imports resolve; prefer +#: ``AwsWrapperMySQLConnectorDialect``. +SqlAlchemyOrmMysqlDialect = AwsWrapperMySQLConnectorDialect + +__all__ = ["SqlAlchemyOrmMysqlDialect"] From e674109042ed88d78aa678aae55cb164cc6960ac Mon Sep 17 00:00:00 2001 From: karezche <64801825+karenc-bq@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:52:27 -0700 Subject: [PATCH 4/4] chore: address review comment --- .../sqlalchemy/mysql_orm_dialect.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/aws_advanced_python_wrapper/sqlalchemy/mysql_orm_dialect.py b/aws_advanced_python_wrapper/sqlalchemy/mysql_orm_dialect.py index bcdd6f605..8914e82f6 100644 --- a/aws_advanced_python_wrapper/sqlalchemy/mysql_orm_dialect.py +++ b/aws_advanced_python_wrapper/sqlalchemy/mysql_orm_dialect.py @@ -25,16 +25,8 @@ to subclass it, or to register it by hand — which would otherwise break with ``ModuleNotFoundError`` on upgrade. -One behavioural difference is worth knowing, though it is not a loss of -capability. 3.0.0's ``create_connect_args`` hard-coded -``plugins = "aurora_connection_tracker,failover_v2"`` whenever ``wrapper_plugins`` -was absent from the URL. The replacement class does not set ``plugins`` at all, -so the wrapper's own default chain applies instead. For mysql-connector that -default is ``initial_connection,aurora_connection_tracker,failover_v2`` — a -superset of what 3.0.0 injected — so failover remains enabled by default. An -application that relied on the old value *exactly* (for instance to keep -``initial_connection`` out of the chain) should now pass ``wrapper_plugins`` in -the URL, or ``plugins`` via ``connect_args``, explicitly. +Note that starting v3.1.0, the plugin chain is no longer hardcoded to `aurora_connection_tracker,failover_v2` +if the connection property `wrapper_plugins` is not set, and will use the default plugin chain instead. """ import warnings