From 03212e5aefb3d5c912d25274f447eb9c493f7268 Mon Sep 17 00:00:00 2001 From: jonathan vanasco Date: Fri, 12 Nov 2021 12:45:54 -0500 Subject: [PATCH] add context for warnings emitted from configure_mappers(), autoflush() Improved the notification of warnings that are emitted within the configure mappers or flush process, which are often invoked as part of a different operation, to add additional context to the message that indicates one of these operations as the source of the warning within operations that may not be obviously related. Fixes: #7305 Change-Id: I79da7a6a5d4cf67d57615d0ffc2b8d8454011c84 --- doc/build/changelog/unreleased_20/7305.rst | 9 ++ lib/sqlalchemy/orm/mapper.py | 6 ++ lib/sqlalchemy/orm/session.py | 6 ++ lib/sqlalchemy/util/langhelpers.py | 47 ++++++-- test/orm/test_utils.py | 120 +++++++++++++++++++++ test/orm/test_versioning.py | 8 +- 6 files changed, 185 insertions(+), 11 deletions(-) create mode 100644 doc/build/changelog/unreleased_20/7305.rst diff --git a/doc/build/changelog/unreleased_20/7305.rst b/doc/build/changelog/unreleased_20/7305.rst new file mode 100644 index 0000000000..5b71f05646 --- /dev/null +++ b/doc/build/changelog/unreleased_20/7305.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: utils, bug + :tickets: 7305 + + Improved the notification of warnings that are emitted within the configure + mappers or flush process, which are often invoked as part of a different + operation, to add additional context to the message that indicates one of + these operations as the source of the warning within operations that may + not be obviously related. diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 141b5c27a4..dd3aef8d69 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -2311,6 +2311,12 @@ class Mapper( "columns get mapped." % (key, self, column.key, prop) ) + @util.langhelpers.tag_method_for_warnings( + "This warning originated from the `configure_mappers()` process, " + "which was invoked automatically in response to a user-initiated " + "operation.", + sa_exc.SAWarning, + ) def _check_configure(self) -> None: if self.registry._new_mappers: _configure_registries({self.registry}, cascade=True) diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index 5bcb22a083..06e84dbcfc 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -2872,6 +2872,12 @@ class Session(_SessionClassMethods, EventTarget): finally: self.autoflush = autoflush + @util.langhelpers.tag_method_for_warnings( + "This warning originated from the Session 'autoflush' process, " + "which was invoked automatically in response to a user-initiated " + "operation.", + sa_exc.SAWarning, + ) def _autoflush(self) -> None: if self.autoflush and not self._flushing: try: diff --git a/lib/sqlalchemy/util/langhelpers.py b/lib/sqlalchemy/util/langhelpers.py index 7671480452..01f083bec2 100644 --- a/lib/sqlalchemy/util/langhelpers.py +++ b/lib/sqlalchemy/util/langhelpers.py @@ -24,6 +24,7 @@ import sys import textwrap import threading import types +from types import CodeType from typing import Any from typing import Callable from typing import cast @@ -1825,6 +1826,19 @@ def warn_limited(msg: str, args: Sequence[Any]) -> None: _warnings_warn(msg, exc.SAWarning) +_warning_tags: Dict[CodeType, Tuple[str, Type[Warning]]] = {} + + +def tag_method_for_warnings( + message: str, category: Type[Warning] +) -> Callable[[_F], _F]: + def go(fn): + _warning_tags[fn.__code__] = (message, category) + return fn + + return go + + _not_sa_pattern = re.compile(r"^(?:sqlalchemy\.(?!testing)|alembic\.)") @@ -1846,14 +1860,33 @@ def _warnings_warn( # ok, but don't crash stacklevel = 0 else: - # using __name__ here requires that we have __name__ in the - # __globals__ of the decorated string functions we make also. - # we generate this using {"__name__": fn.__module__} - while frame is not None and re.match( - _not_sa_pattern, frame.f_globals.get("__name__", "") - ): + stacklevel_found = warning_tag_found = False + while frame is not None: + # using __name__ here requires that we have __name__ in the + # __globals__ of the decorated string functions we make also. + # we generate this using {"__name__": fn.__module__} + if not stacklevel_found and not re.match( + _not_sa_pattern, frame.f_globals.get("__name__", "") + ): + # stop incrementing stack level if an out-of-SQLA line + # were found. + stacklevel_found = True + + # however, for the warning tag thing, we have to keep + # scanning up the whole traceback + + if frame.f_code in _warning_tags: + warning_tag_found = True + (_prefix, _category) = _warning_tags[frame.f_code] + category = category or _category + message = f"{message} ({_prefix})" + frame = frame.f_back # type: ignore[assignment] - stacklevel += 1 + + if not stacklevel_found: + stacklevel += 1 + elif stacklevel_found and warning_tag_found: + break if category is not None: warnings.warn(message, category, stacklevel=stacklevel + 1) diff --git a/test/orm/test_utils.py b/test/orm/test_utils.py index c829582fdf..3bd0230dea 100644 --- a/test/orm/test_utils.py +++ b/test/orm/test_utils.py @@ -1,4 +1,8 @@ +import re + from sqlalchemy import Column +from sqlalchemy import event +from sqlalchemy import ForeignKey from sqlalchemy import inspect from sqlalchemy import Integer from sqlalchemy import MetaData @@ -10,6 +14,7 @@ from sqlalchemy.ext.hybrid import hybrid_method from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm import aliased from sqlalchemy.orm import clear_mappers +from sqlalchemy.orm import relationship from sqlalchemy.orm import Session from sqlalchemy.orm import synonym from sqlalchemy.orm import util as orm_util @@ -30,6 +35,121 @@ from test.orm import _fixtures from .inheritance import _poly_fixtures +class ContextualWarningsTest(fixtures.TestBase): + """ + Test for #7305 + + """ + + @testing.fixture + def plain_fixture(cls, decl_base): + class Foo(decl_base): + __tablename__ = "foo" + id = Column(Integer, primary_key=True) + + decl_base.metadata.create_all(testing.db) + return Foo + + @testing.fixture + def overlap_fixture(cls, decl_base): + class Foo(decl_base): + __tablename__ = "foo" + id = Column(Integer, primary_key=True) + bars = relationship( + "Bar", + primaryjoin="Foo.id==Bar.foo_id", + ) + + class Bar(decl_base): + __tablename__ = "bar" + id = Column(Integer, primary_key=True) + foo_id = Column(Integer, ForeignKey("foo.id")) + foos = relationship( + "Foo", + primaryjoin="Bar.foo_id==Foo.id", + ) + + return Foo, Bar + + def test_configure_mappers_explicit(self, overlap_fixture, decl_base): + with expect_warnings( + re.escape( + "relationship 'Bar.foos' will copy column foo.id to column " + "bar.foo_id, which conflicts with relationship(s): 'Foo.bars' " + "(copies foo.id to bar.foo_id). " + ), + raise_on_any_unexpected=True, + ): + decl_base.registry.configure() + + def test_configure_mappers_implicit_aliased(self, overlap_fixture): + Foo, Bar = overlap_fixture + with expect_warnings( + re.escape( + "relationship 'Bar.foos' will copy column foo.id " + "to column bar.foo_id, which conflicts with" + ) + + ".*" + + re.escape( + "(This warning originated from the `configure_mappers()` " + "process, which was " + "invoked automatically in response to a user-initiated " + "operation.)" + ), + raise_on_any_unexpected=True, + ): + FooAlias = aliased(Foo) + assert hasattr(FooAlias, "bars") + + def test_configure_mappers_implicit_instantiate(self, overlap_fixture): + Foo, Bar = overlap_fixture + with expect_warnings( + re.escape( + "relationship 'Bar.foos' will copy column foo.id " + "to column bar.foo_id, which conflicts with" + ) + + ".*" + + re.escape( + "(This warning originated from the `configure_mappers()` " + "process, which was " + "invoked automatically in response to a user-initiated " + "operation.)" + ), + raise_on_any_unexpected=True, + ): + foo = Foo() + assert hasattr(foo, "bars") + + def test_autoflush_implicit(self, plain_fixture): + Foo = plain_fixture + + sess = fixture_session() + + @event.listens_for(Foo, "before_insert") + def emit_a_warning(mapper, connection, state): + sess.add(Foo()) + + sess.add(Foo()) + + with expect_warnings( + re.escape( + "Usage of the 'Session.add()' operation is not " + "currently supported within the execution stage of the flush " + "process. Results may not be consistent. Consider using " + "alternative event listeners or connection-level operations " + "instead." + ) + + ".*" + + re.escape( + "(This warning originated from the Session 'autoflush' " + "process, which was invoked automatically in response to a " + "user-initiated operation.)" + ), + raise_on_any_unexpected=True, + ): + sess.execute(select(Foo)) + + class AliasedClassTest(fixtures.MappedTest, AssertsCompiledSQL): __dialect__ = "default" diff --git a/test/orm/test_versioning.py b/test/orm/test_versioning.py index 84e5a83b07..1a1801311b 100644 --- a/test/orm/test_versioning.py +++ b/test/orm/test_versioning.py @@ -53,13 +53,13 @@ def conditional_sane_rowcount_warnings( ): if update: warnings += ( - "Dialect .* does not support updated rowcount - " - "versioning cannot be verified.", + "Dialect .* does not support " + "updated rowcount - versioning cannot be verified.", ) if delete: warnings += ( - "Dialect .* does not support deleted rowcount - " - "versioning cannot be verified.", + "Dialect .* does not support " + "deleted rowcount - versioning cannot be verified.", ) with expect_warnings(*warnings): -- 2.47.2