Skip to content

event listener not working for sessionmaker, or any Session subclass #2424

New issue
New issue
Closed
Closed
event listener not working for sessionmaker, or any Session subclass#2424
Labels
blockerissue that must be resolved asap as it is preventing things from workingbugSomething isn't workingorm
Milestone
 
0.7.6
@sqlalchemy-bot

Description

@sqlalchemy-bot
sqlalchemy-bot
opened on Mar 7, 2012
Collaborator

Migrated issue, originally created by Michael Bayer (@zzzeek)

import sqlalchemy as sa
import sqlalchemy.exc as saexc
import sqlalchemy.ext.declarative as sadec
import sqlalchemy.sql as sasql
import sqlalchemy.orm as saorm

@sa.event.listens_for(saorm.Session, 'before_flush')
def handler1(session, flush_context, instances):
    print 'handler1'

engine = sa.create_engine('sqlite://')
meta = sa.MetaData()
Base = sadec.declarative_base(metadata=meta)

class MySession(saorm.Session):
    pass

# works
#sess = saorm.Session(bind=engine, autoflush=False)

# fails
#sessmaker = saorm.sessionmaker(bind = engine, autoflush = False)
#sess = sessmaker()

# fails
sess = MySession(bind=engine, autoflush=False)

class Anything(Base):
    __tablename__ = 'families'

    id = sa.Column(sa.Integer, primary_key=True)

meta.create_all(bind=engine)

@sa.event.listens_for(saorm.Session, 'before_flush')
def handler2(session, flush_context, instances):
    print 'handler2'

f = Anything()
sess.add(f)
sess.commit()

Activity

sqlalchemy-bot

sqlalchemy-bot commented on Mar 7, 2012

@sqlalchemy-bot
sqlalchemy-bot
on Mar 7, 2012
CollaboratorAuthor

Michael Bayer (@zzzeek) wrote:

this is basically an issue with class generation in the event system.

sqlalchemy-bot

sqlalchemy-bot commented on Mar 7, 2012

@sqlalchemy-bot
sqlalchemy-bot
on Mar 7, 2012
CollaboratorAuthor

Michael Bayer (@zzzeek) wrote:

this is not the fix but illustrates the issue, that new subclasses made after the fact don't get populated in the parent._clslevel mapping.

diff -r be71c73f614f9c316c0c02a98505a8c03d5ef788 lib/sqlalchemy/event.py
--- a/lib/sqlalchemy/event.py	Mon Mar 05 15:20:07 2012 -0500
+++ b/lib/sqlalchemy/event.py	Wed Mar 07 11:28:21 2012 -0500
@@ -252,11 +252,20 @@
     _exec_once = False
 
     def __init__(self, parent, target_cls):
-        self.parent_listeners = parent._clslevel[target_cls](target_cls)
+        #self.parent_listeners = parent._clslevel[target_cls](target_cls)
+        self.target_cls = target_cls
+        self.parent = parent
         self.name = parent.__name__
         self.listeners = [        self.propagate = set()
 
+    @property
+    def parent_listeners(self):
+        lis = set()
+        for cls in self.target_cls.__mro__:
+            lis.update(self.parent._clslevel[cls](]
))
+        return list(lis)
+
     def exec_once(self, *args, **kw):
         """Execute this event, but only if it has not been
         executed already for this collection."""
sqlalchemy-bot

sqlalchemy-bot commented on Mar 7, 2012

@sqlalchemy-bot
sqlalchemy-bot
on Mar 7, 2012
CollaboratorAuthor

Michael Bayer (@zzzeek) wrote:

here's a patch that fixes, though we're having a heisenbug in test.sql.test_metadata:CatchAllEventsTest with it. Running test_metadata by itself passes, running all of -w sql, the patch fails, so something with the "event cleanup" code might be getting in the way here.

diff -r be71c73f614f9c316c0c02a98505a8c03d5ef788 lib/sqlalchemy/event.py
--- a/lib/sqlalchemy/event.py	Mon Mar 05 15:20:07 2012 -0500
+++ b/lib/sqlalchemy/event.py	Wed Mar 07 11:54:00 2012 -0500
@@ -206,21 +206,19 @@
         assert isinstance(target, type), \
                 "Class-level Event targets must be classes."
 
-        stack = [target](target)
-        while stack:
-            cls = stack.pop(0)
-            stack.extend(cls.__subclasses__())
-            self._clslevel[cls](cls).insert(0, obj)
+        self._clslevel[target](target).insert(0, obj)
 
     def append(self, obj, target, propagate):
         assert isinstance(target, type), \
                 "Class-level Event targets must be classes."
 
-        stack = [target](target)
-        while stack:
-            cls = stack.pop(0)
-            stack.extend(cls.__subclasses__())
-            self._clslevel[cls](cls).append(obj)
+        self._clslevel[target](target).append(obj)
+
+    def update_subclass(self, target):
+        clslevel = self._clslevel[target](target)
+        for cls in target.__mro__:
+            if cls in self._clslevel:
+                clslevel.extend(self._clslevel[cls](cls))
 
     def remove(self, obj, target):
         stack = [target](target)
@@ -252,6 +250,8 @@
     _exec_once = False
 
     def __init__(self, parent, target_cls):
+        if target_cls not in parent._clslevel:
+            parent.update_subclass(target_cls)
         self.parent_listeners = parent._clslevel[target_cls](target_cls)
         self.name = parent.__name__
         self.listeners = []
sqlalchemy-bot

sqlalchemy-bot commented on Mar 7, 2012

@sqlalchemy-bot
sqlalchemy-bot
on Mar 7, 2012
CollaboratorAuthor

Michael Bayer (@zzzeek) wrote:

yeah it's the "clear" - which needs to hold onto those lists that are already created. if we just make that check for parent._cls more liberal then it's OK:

diff -r be71c73f614f9c316c0c02a98505a8c03d5ef788 lib/sqlalchemy/event.py
--- a/lib/sqlalchemy/event.py	Mon Mar 05 15:20:07 2012 -0500
+++ b/lib/sqlalchemy/event.py	Wed Mar 07 11:59:36 2012 -0500
@@ -206,21 +206,19 @@
         assert isinstance(target, type), \
                 "Class-level Event targets must be classes."
 
-        stack = [target](target)
-        while stack:
-            cls = stack.pop(0)
-            stack.extend(cls.__subclasses__())
-            self._clslevel[cls](cls).insert(0, obj)
+        self._clslevel[target](target).insert(0, obj)
 
     def append(self, obj, target, propagate):
         assert isinstance(target, type), \
                 "Class-level Event targets must be classes."
 
-        stack = [target](target)
-        while stack:
-            cls = stack.pop(0)
-            stack.extend(cls.__subclasses__())
-            self._clslevel[cls](cls).append(obj)
+        self._clslevel[target](target).append(obj)
+
+    def update_subclass(self, target):
+        clslevel = self._clslevel[target](target)
+        for cls in target.__mro__:
+            if cls in self._clslevel:
+                clslevel.extend(self._clslevel[cls](cls))
 
     def remove(self, obj, target):
         stack = [target](target)
@@ -252,6 +250,8 @@
     _exec_once = False
 
     def __init__(self, parent, target_cls):
+        if not parent._clslevel[target_cls](target_cls):
+            parent.update_subclass(target_cls)
         self.parent_listeners = parent._clslevel[target_cls](target_cls)
         self.name = parent.__name__
         self.listeners = []

this incurs a lot of unnecessary, instance-level calls to update_subclass() though, in the usual case that there's no events set up, so let's keep trying

sqlalchemy-bot

sqlalchemy-bot commented on Mar 7, 2012

@sqlalchemy-bot
sqlalchemy-bot
on Mar 7, 2012
CollaboratorAuthor

Michael Bayer (@zzzeek) wrote:

this one should do it:

diff -r be71c73f614f9c316c0c02a98505a8c03d5ef788 lib/sqlalchemy/event.py
--- a/lib/sqlalchemy/event.py	Mon Mar 05 15:20:07 2012 -0500
+++ b/lib/sqlalchemy/event.py	Wed Mar 07 12:13:36 2012 -0500
@@ -210,7 +210,11 @@
         while stack:
             cls = stack.pop(0)
             stack.extend(cls.__subclasses__())
-            self._clslevel[cls](cls).insert(0, obj)
+            if cls is not target and \
+                cls not in self._clslevel:
+                self.update_subclass(cls)
+            else:
+                self._clslevel[cls](cls).insert(0, obj)
 
     def append(self, obj, target, propagate):
         assert isinstance(target, type), \
@@ -220,7 +224,18 @@
         while stack:
             cls = stack.pop(0)
             stack.extend(cls.__subclasses__())
-            self._clslevel[cls](cls).append(obj)
+            if cls is not target and \
+                cls not in self._clslevel:
+                self.update_subclass(cls)
+            else:
+                self._clslevel[cls](cls).append(obj)
+
+
+    def update_subclass(self, target):
+        clslevel = self._clslevel[target](target)
+        for cls in target.__mro__:
+            if cls in self._clslevel:
+                clslevel.extend(self._clslevel[cls](cls))
 
     def remove(self, obj, target):
         stack = [target](target)
@@ -252,6 +267,8 @@
     _exec_once = False
 
     def __init__(self, parent, target_cls):
+        if target_cls not in parent._clslevel:
+            parent.update_subclass(target_cls)
         self.parent_listeners = parent._clslevel[target_cls](target_cls)
         self.name = parent.__name__
         self.listeners = []
sqlalchemy-bot

sqlalchemy-bot commented on Mar 8, 2012

@sqlalchemy-bot
sqlalchemy-bot
on Mar 8, 2012
CollaboratorAuthor

Michael Bayer (@zzzeek) wrote:

8696a45

sqlalchemy-bot

sqlalchemy-bot commented on Mar 8, 2012

@sqlalchemy-bot
sqlalchemy-bot
on Mar 8, 2012
CollaboratorAuthor

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
sqlalchemy-bot
closed this as completedon Mar 8, 2012
sqlalchemy-bot
added
bugSomething isn't working
blockerissue that must be resolved asap as it is preventing things from working
orm
on Nov 27, 2018
sqlalchemy-bot
added this to the 0.7.6 milestone on Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    blockerissue that must be resolved asap as it is preventing things from workingbugSomething isn't workingorm

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @sqlalchemy-bot

        Issue actions
          event listener not working for sessionmaker, or any Session subclass · Issue #2424 · sqlalchemy/sqlalchemy