improve error message for SQL emitted within after_commit hook; document after_begin alternative?

[Former user]

(original reporter: bkabrda) Hi, I'm having a problem with accessing related objects in event.after_commit hook. The use-case is pretty longish, but it can be reduced to these steps:

1) create models: A many-to-one B, e.g. A has "b = relationship('A')" 2) create an after_commit hook that tries to access A.b on commit of A object 3) run and get: sqlalchemy.exc.InvalidRequestError: This Session's transaction has been rolled back by a nested rollback() call. To begin a new transaction, issue Session.rollback() first.

(I'm sure that there was no rollback, because I tried listening to rollbacks and the listener wasn't run.)

[Mike Bayer]

in the future, you really need to at least make an effort to provide a full test case, as described in the participation guide as you cannot assume your issue is plainly reproducible - and at the very least, paste the stack trace that you've already got on your screen. When there's no test or even a stack trace, it doubles my work.

Such as below, a literal interpretation of your described test shows no issue:

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy import event

Base = declarative_base()

class A(Base):
    __tablename__ = "a"

    id = Column(Integer, primary_key=True)
    data = Column(String)
    b_id = Column(Integer, ForeignKey('b.id'))

    b = relationship("B")

class B(Base):
    __tablename__ = "b"

    id = Column(Integer, primary_key=True)

@event.listens_for(Session, "after_commit")
def run_after_commit(session):
    for obj in session.identity_map.values():
        if isinstance(obj, A):
            print "a.b is:", obj.b

e = create_engine("sqlite://", echo=True)

Base.metadata.create_all(e)

s = Session(e)

a1 = A()
b1 = B()
a1.b = b1
s.add_all([b1](a1,))

s.commit()

However, in this case I do have the advantage of a guess as to where this issue comes up, which is when the "A.b" isn't loaded:

a1 = s.query(A).first()
s.commit()

and really, it's just that the Session isn't in a state where it can emit SQL inside of this hook, so here's the most minimal test case:

from sqlalchemy import event
from sqlalchemy.orm import Session
from sqlalchemy import create_engine

@event.listens_for(Session, "after_commit")
def run_after_commit(session):
    session.execute("select 1")

e = create_engine("sqlite://", echo=True)

s = Session(e)
s.commit()

so this is a pretty generalized limitation of the after_commit() hook. It is possible to set the flags so that you can still emit SQL within this hook, but then that implies that flush and other transactional operations are occurring in a space that isn't accounted for, transaction wise - that is, the commit has ended, but the subsequent transaction hasn't started yet, so this would actually be a much bigger issue, which is that the Session is ambiguous about its own state.

So for after_commit() the best I can do here is a nicer error message. For the use case of, "I want to do something with the database after the transaction has ended", you really want to be running that SQL in the next transaction, which means you should probably use the after_begin() hook instead.

[Former user]

(original author: bkabrda) Ok, sorry for the bad report, I'll improve it for the next time...

The problem with after_begin is, that you can no more access objects changed in last commit. E.g. if I want to do something with the changed object, AFAICS the only way is to use after_commit. So it seems that preloading the related object is the only way here, am I right?

Thanks.

[Mike Bayer]

if you want to detect which objects were affected, that's something that's local to the "flush", not really the commit. So there's two hooks, "after_flush" and "after_flush_postexec" - the former hook is where you can still look inside of session.new, session.dirty, and session.deleted to detect those objects which were operated upon. Also, there can be any number of flushes before the commit happens so that would imply the after_flush hook is what you need in this case.

[Former user]

(original author: bkabrda) Great, that's exactly what I needed. Thanks a lot!

[Mike Bayer]

07cc4ecf05aeb746a54ce51671d7a4b2669141e2

[Mike Bayer]

Removing milestone: 0.8.0final (automated comment)