]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
add context for warnings emitted from configure_mappers(), autoflush()
authorjonathan vanasco <jonathan@2xlp.com>
Fri, 12 Nov 2021 17:45:54 +0000 (12:45 -0500)
committerMike Bayer <mike_mp@zzzcomputing.com>
Wed, 25 Jan 2023 03:03:06 +0000 (22:03 -0500)
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 [new file with mode: 0644]
lib/sqlalchemy/orm/mapper.py
lib/sqlalchemy/orm/session.py
lib/sqlalchemy/util/langhelpers.py
test/orm/test_utils.py
test/orm/test_versioning.py

diff --git a/doc/build/changelog/unreleased_20/7305.rst b/doc/build/changelog/unreleased_20/7305.rst
new file mode 100644 (file)
index 0000000..5b71f05
--- /dev/null
@@ -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.
index 141b5c27a4c519c4f15e46d0e1d99faaa8b8a1a9..dd3aef8d69785a029662e4ee34bebff3e5e6be68 100644 (file)
@@ -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)
index 5bcb22a083b298dea085885c976ac893a8605ae5..06e84dbcfc354ca8ca25d1b8bb8b49bf1767356c 100644 (file)
@@ -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:
index 7671480452052267e1f80ac79ee93f28b5bc3983..01f083bec2d21fc86bf4de61e1ebb3c2817a9068 100644 (file)
@@ -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)
index c829582fdfb579fc4f34e501e28d67ad3bf3de75..3bd0230dea67482f9993b49c7268d93fc9870b91 100644 (file)
@@ -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"
 
index 84e5a83b07e3f7888cbc9180606ed8c35c5702f1..1a1801311b3841bc062523d482aaa46d0fbe04f9 100644 (file)
@@ -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):