After Session.commit() the enclosing transaction is not activated back

[Former user]

In {{{session.py:651}}} we have the {{{Session.commit}}} method:

    def commit(self):
        """Flush pending changes and commit the current transaction.

        If no transaction is in progress, this method raises an
        InvalidRequestError.

        If a subtransaction is in effect (which occurs when begin() is called
        multiple times), the subtransaction will be closed, and the next call
        to ``commit()`` will operate on the enclosing transaction.

        For a session configured with autocommit=False, a new transaction will
        be begun immediately after the commit, but note that the newly begun
        transaction does *not* use any connection resources until the first
        SQL is actually emitted.

        """
        if self.transaction is None:
            if not self.autocommit:
                self.begin()
            else:
                raise sa_exc.InvalidRequestError("No transaction is begun.")

        self.transaction.commit()

Shouldn't the last line be like the following?

        self.transaction = self.transaction.commit()

The {{{SessionTransaction.commit}}} method returns the parent transaction AFAIK.

Sorry if I'm wrong!

[Mike Bayer]

it happens here (line 426 approx):

    def close(self):
        self.session.transaction = self._parent
        if self._parent is None:
            for connection, transaction, autoclose in set(self._connections.values()):
                if autoclose:
                    connection.close()
                else:
                    transaction.close()
            if not self.session.autocommit:
                self.session.begin()
        self._deactivate()
        self.session = None
        self._connections = None

i.e. the SessionTransaction manages the session's state.

if you'd like, you can try putting "self.transaction = self.transaction.commit()/rollback()" in the appropriate places and remove the attribute set within SessionTransaction.close(). That does seem to be clearer and it probably used to be that way...which implies there may be a rationale for the current approach (though I'm not sure that there is, Ants Aasma refactored the code at some point).

In particular the unit tests in test/orm/session.py and test/orm/transaction.py, when run against a fully supporting database like postgres, will reveal any areas of functionality that are dependent on the SessoinTransaction.close().

[Former user]

I'm not sure if it is worth the effort, especially that, as you say, it was done this way for some reason.

I propose to add a comment like this:

patch -p1 <<EOF
--- a/orm/session.py
+++ b/orm/session.py
@@ -670,6 +670,8 @@ class Session(object):
             else:
                 raise sa_exc.InvalidRequestError("No transaction is begun.")

+        # self.transaction is set to the enclosed transaction by
+        # SessionTransaction.commit(), if needed.
         self.transaction.commit()

     def prepare(self):

EOF

[Mike Bayer]

there's no bug here so closing

[Mike Bayer]

Removing milestone: 0.6.xx (automated comment)