From 112c0a5428a18bd372467e3b6a3c52c8d65091a6 Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Sun, 18 Jan 2026 12:24:38 +0000 Subject: [PATCH 1/9] Remove duplicate license header in abstract.py Co-Authored-By: Claude Opus 4.5 --- sqlalchemy_bind_manager/_repository/abstract.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/sqlalchemy_bind_manager/_repository/abstract.py b/sqlalchemy_bind_manager/_repository/abstract.py index 1c1a5b2..500cc16 100644 --- a/sqlalchemy_bind_manager/_repository/abstract.py +++ b/sqlalchemy_bind_manager/_repository/abstract.py @@ -18,15 +18,6 @@ # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER # DEALINGS IN THE SOFTWARE. -# -# Permission is hereby granted, free of charge, to any person obtaining a -# copy of this software and associated documentation files (the "Software"), -# to deal in the Software without restriction, including without limitation -# the rights to use, copy, modify, merge, publish, distribute, sublicense, -# and/or sell copies of the Software, and to permit persons to whom the -# Software is furnished to do so, subject to the following conditions: -# -# from typing import ( Any, Iterable, From 60b57192c5a8a3cb6e6a464b64eb9f26992e5d62 Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Sun, 18 Jan 2026 12:25:08 +0000 Subject: [PATCH 2/9] Remove unnecessary partial wrapper in order_by registry The partial() wrapper was not needed since asc and desc can be referenced directly as callables. Co-Authored-By: Claude Opus 4.5 --- sqlalchemy_bind_manager/_repository/base_repository.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/sqlalchemy_bind_manager/_repository/base_repository.py b/sqlalchemy_bind_manager/_repository/base_repository.py index a4e09a5..dc3cf63 100644 --- a/sqlalchemy_bind_manager/_repository/base_repository.py +++ b/sqlalchemy_bind_manager/_repository/base_repository.py @@ -19,10 +19,8 @@ # DEALINGS IN THE SOFTWARE. from abc import ABC -from functools import partial from typing import ( Any, - Callable, Dict, Generic, Iterable, @@ -131,9 +129,9 @@ def _filter_order_by( :param order_by: a list of columns, or tuples (column, direction) :return: The filtered query """ - _partial_registry: Dict[Literal["asc", "desc"], Callable] = { - "desc": partial(desc), - "asc": partial(asc), + _order_funcs: Dict[Literal["asc", "desc"], type] = { + "desc": desc, + "asc": asc, } for value in order_by: @@ -143,7 +141,7 @@ def _filter_order_by( else: self._validate_mapped_property(value[0]) stmt = stmt.order_by( - _partial_registry[value[1]](getattr(self._model, value[0])) + _order_funcs[value[1]](getattr(self._model, value[0])) ) return stmt From a4c9cbcd69d11ad3d3e20eb4838cca9384e22699 Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Sun, 18 Jan 2026 12:25:18 +0000 Subject: [PATCH 3/9] Remove deprecated future=True engine option The future=True option is deprecated in SQLAlchemy 2.0 and is now the default behavior. Co-Authored-By: Claude Opus 4.5 --- sqlalchemy_bind_manager/_bind_manager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sqlalchemy_bind_manager/_bind_manager.py b/sqlalchemy_bind_manager/_bind_manager.py index a509750..ae89e59 100644 --- a/sqlalchemy_bind_manager/_bind_manager.py +++ b/sqlalchemy_bind_manager/_bind_manager.py @@ -115,7 +115,6 @@ def __init_bind(self, name: str, config: SQLAlchemyConfig): engine_options: dict = config.engine_options or {} engine_options.setdefault("echo", False) - engine_options.setdefault("future", True) session_options: dict = config.session_options or {} session_options.setdefault("expire_on_commit", False) From a4e0487058d227bb40d603bee4ee293280418862 Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Sun, 18 Jan 2026 12:25:27 +0000 Subject: [PATCH 4/9] Remove redundant or [] fallback in cursor_paginated_find The list comprehension already produces a list, making the or [] fallback unnecessary. Co-Authored-By: Claude Opus 4.5 --- sqlalchemy_bind_manager/_repository/async_.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlalchemy_bind_manager/_repository/async_.py b/sqlalchemy_bind_manager/_repository/async_.py index e94ee5f..e332a54 100644 --- a/sqlalchemy_bind_manager/_repository/async_.py +++ b/sqlalchemy_bind_manager/_repository/async_.py @@ -274,7 +274,7 @@ async def cursor_paginated_find( ).scalar() or 0 result_items = [ x for x in (await session.execute(paginated_stmt)).scalars() - ] or [] + ] return CursorPaginatedResultPresenter.build_result( result_items=result_items, From 0ead23add050f211fafbac51ecc4b18dba1a9560 Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Sun, 18 Jan 2026 12:25:36 +0000 Subject: [PATCH 5/9] Add exception handling to SessionHandler.__del__ The scoped_session.remove() call could fail if the database connection is already closed during garbage collection. Wrapping in try/except with debug logging prevents errors during interpreter shutdown. Co-Authored-By: Claude Opus 4.5 --- sqlalchemy_bind_manager/_session_handler.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/sqlalchemy_bind_manager/_session_handler.py b/sqlalchemy_bind_manager/_session_handler.py index 19c1c53..35989b5 100644 --- a/sqlalchemy_bind_manager/_session_handler.py +++ b/sqlalchemy_bind_manager/_session_handler.py @@ -19,6 +19,7 @@ # DEALINGS IN THE SOFTWARE. import asyncio +import logging from contextlib import asynccontextmanager, contextmanager from typing import AsyncIterator, Iterator @@ -34,6 +35,8 @@ ) from sqlalchemy_bind_manager.exceptions import UnsupportedBindError +logger = logging.getLogger(__name__) + class SessionHandler: scoped_session: scoped_session @@ -45,8 +48,11 @@ def __init__(self, bind: SQLAlchemyBind): self.scoped_session = scoped_session(bind.session_class) def __del__(self): - if getattr(self, "scoped_session", None): - self.scoped_session.remove() + try: + if getattr(self, "scoped_session", None): + self.scoped_session.remove() + except Exception: + logger.debug("Failed to remove scoped session", exc_info=True) @contextmanager def get_session(self, read_only: bool = False) -> Iterator[Session]: From e6dee271035898355d22623b6c84f9c535c7a91c Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Sun, 18 Jan 2026 12:39:09 +0000 Subject: [PATCH 6/9] Use any() for more efficient model validation Replaces list comprehension with any() generator expression to short-circuit on first invalid model instead of iterating all items. Co-Authored-By: Claude Opus 4.5 --- sqlalchemy_bind_manager/_repository/base_repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlalchemy_bind_manager/_repository/base_repository.py b/sqlalchemy_bind_manager/_repository/base_repository.py index dc3cf63..396ed69 100644 --- a/sqlalchemy_bind_manager/_repository/base_repository.py +++ b/sqlalchemy_bind_manager/_repository/base_repository.py @@ -349,7 +349,7 @@ def _model_pk(self) -> str: return primary_keys[0].name def _fail_if_invalid_models(self, objects: Iterable[MODEL]) -> None: - if [x for x in objects if not isinstance(x, self._model)]: + if any(not isinstance(x, self._model) for x in objects): raise InvalidModelError( "Cannot handle models not belonging to this repository" ) From 662add1bbf07d439c2b2cc521aa979cff331b853 Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Sun, 18 Jan 2026 12:40:08 +0000 Subject: [PATCH 7/9] Extract shared primary key utility function Consolidates duplicated PK inspection logic from BaseRepository._model_pk() and result_presenters._pk_from_result_object() into a single get_model_pk_name() function in common.py. Co-Authored-By: Claude Opus 4.5 --- .../_repository/base_repository.py | 9 +++------ sqlalchemy_bind_manager/_repository/common.py | 16 +++++++++++++++- .../_repository/result_presenters.py | 13 ++----------- .../result_presenters/test_composite_pk.py | 6 +++--- 4 files changed, 23 insertions(+), 21 deletions(-) diff --git a/sqlalchemy_bind_manager/_repository/base_repository.py b/sqlalchemy_bind_manager/_repository/base_repository.py index 396ed69..d0059b9 100644 --- a/sqlalchemy_bind_manager/_repository/base_repository.py +++ b/sqlalchemy_bind_manager/_repository/base_repository.py @@ -31,7 +31,7 @@ Union, ) -from sqlalchemy import asc, desc, func, inspect, select +from sqlalchemy import asc, desc, func, select from sqlalchemy.orm import Mapper, aliased, class_mapper, lazyload from sqlalchemy.orm.exc import UnmappedClassError from sqlalchemy.sql import Select @@ -41,6 +41,7 @@ from .common import ( MODEL, CursorReference, + get_model_pk_name, ) @@ -342,11 +343,7 @@ def _model_pk(self) -> str: :return: """ - primary_keys = inspect(self._model).primary_key # type: ignore - if len(primary_keys) > 1: - raise NotImplementedError("Composite primary keys are not supported.") - - return primary_keys[0].name + return get_model_pk_name(self._model) def _fail_if_invalid_models(self, objects: Iterable[MODEL]) -> None: if any(not isinstance(x, self._model) for x in objects): diff --git a/sqlalchemy_bind_manager/_repository/common.py b/sqlalchemy_bind_manager/_repository/common.py index a2abeb7..b07e4f8 100644 --- a/sqlalchemy_bind_manager/_repository/common.py +++ b/sqlalchemy_bind_manager/_repository/common.py @@ -18,15 +18,29 @@ # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER # DEALINGS IN THE SOFTWARE. -from typing import Generic, List, TypeVar, Union +from typing import Generic, List, Type, TypeVar, Union from uuid import UUID from pydantic import BaseModel, StrictInt, StrictStr +from sqlalchemy import inspect MODEL = TypeVar("MODEL") PRIMARY_KEY = Union[str, int, tuple, dict, UUID] +def get_model_pk_name(model_class: Type) -> str: + """Retrieves the primary key column name from a SQLAlchemy model class. + + :param model_class: A SQLAlchemy model class + :return: The name of the primary key column + :raises NotImplementedError: If the model has composite primary keys + """ + primary_keys = inspect(model_class).primary_key # type: ignore + if len(primary_keys) > 1: + raise NotImplementedError("Composite primary keys are not supported.") + return primary_keys[0].name + + class PageInfo(BaseModel): """ Paginated query metadata. diff --git a/sqlalchemy_bind_manager/_repository/result_presenters.py b/sqlalchemy_bind_manager/_repository/result_presenters.py index 5c08e7e..7792a92 100644 --- a/sqlalchemy_bind_manager/_repository/result_presenters.py +++ b/sqlalchemy_bind_manager/_repository/result_presenters.py @@ -21,8 +21,6 @@ from math import ceil from typing import List, Union -from sqlalchemy import inspect - from .common import ( MODEL, CursorPageInfo, @@ -30,6 +28,7 @@ CursorReference, PageInfo, PaginatedResult, + get_model_pk_name, ) @@ -93,7 +92,7 @@ def _build_no_cursor_result( has_next_page = len(result_items) > items_per_page if has_next_page: result_items = result_items[0:items_per_page] - reference_column = _pk_from_result_object(result_items[0]) + reference_column = get_model_pk_name(type(result_items[0])) return CursorPaginatedResult( items=result_items, @@ -237,11 +236,3 @@ def build_result( has_previous_page=has_previous_page, ), ) - - -def _pk_from_result_object(model) -> str: - primary_keys = inspect(type(model)).primary_key # type: ignore - if len(primary_keys) > 1: - raise NotImplementedError("Composite primary keys are not supported.") - - return primary_keys[0].name diff --git a/tests/repository/result_presenters/test_composite_pk.py b/tests/repository/result_presenters/test_composite_pk.py index 20232f4..81f1286 100644 --- a/tests/repository/result_presenters/test_composite_pk.py +++ b/tests/repository/result_presenters/test_composite_pk.py @@ -2,15 +2,15 @@ import pytest -from sqlalchemy_bind_manager._repository.result_presenters import _pk_from_result_object +from sqlalchemy_bind_manager._repository.common import get_model_pk_name def test_exception_raised_if_multiple_primary_keys(): with ( patch( - "sqlalchemy_bind_manager._repository.result_presenters.inspect", + "sqlalchemy_bind_manager._repository.common.inspect", return_value=Mock(primary_key=["1", "2"]), ), pytest.raises(NotImplementedError), ): - _pk_from_result_object("irrelevant") + get_model_pk_name(str) From 04e9afc3946070299a52dd21ec86964e8ce71c7e Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Sun, 18 Jan 2026 12:43:28 +0000 Subject: [PATCH 8/9] Add test for SessionHandler.__del__ exception handling Verifies that exceptions from scoped_session.remove() are caught and logged during cleanup, preventing errors during garbage collection. Co-Authored-By: Claude Opus 4.5 --- tests/session_handler/test_session_lifecycle.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/session_handler/test_session_lifecycle.py b/tests/session_handler/test_session_lifecycle.py index 7ed373c..8a91af1 100644 --- a/tests/session_handler/test_session_lifecycle.py +++ b/tests/session_handler/test_session_lifecycle.py @@ -32,6 +32,21 @@ def test_sync_session_is_removed_on_cleanup(sa_manager): mocked_remove.assert_called_once() +def test_sync_session_cleanup_handles_exception(sa_manager): + """Test that __del__ gracefully handles exceptions from scoped_session.remove().""" + sh = SessionHandler(sa_manager.get_bind("sync")) + + with patch.object( + sh.scoped_session, + "remove", + side_effect=Exception("Connection already closed"), + ) as mocked_remove: + # This should not raise - the exception should be caught and logged + sh.__del__() + + mocked_remove.assert_called_once() + + @pytest.mark.parametrize("read_only_flag", [True, False]) async def test_commit_is_called_only_if_not_read_only( read_only_flag, From 8bfd0fef1bbb260b0a2d5743a4a68f174af7ec5e Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Sun, 18 Jan 2026 12:45:08 +0000 Subject: [PATCH 9/9] Fix type annotation for order_by function registry Use Callable instead of type since asc/desc are functions, not types. Co-Authored-By: Claude Opus 4.5 --- sqlalchemy_bind_manager/_repository/base_repository.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sqlalchemy_bind_manager/_repository/base_repository.py b/sqlalchemy_bind_manager/_repository/base_repository.py index d0059b9..be09792 100644 --- a/sqlalchemy_bind_manager/_repository/base_repository.py +++ b/sqlalchemy_bind_manager/_repository/base_repository.py @@ -21,6 +21,7 @@ from abc import ABC from typing import ( Any, + Callable, Dict, Generic, Iterable, @@ -130,7 +131,7 @@ def _filter_order_by( :param order_by: a list of columns, or tuples (column, direction) :return: The filtered query """ - _order_funcs: Dict[Literal["asc", "desc"], type] = { + _order_funcs: Dict[Literal["asc", "desc"], Callable] = { "desc": desc, "asc": asc, }