Keeping soft deletion atomic in spite of model methods calling commit

[charlax]

Hi,

I'm trying to implement soft deletion in my app. I'd like to keep things atomic so that if an error happens during the soft deletion process, I don't get half deleted stuff.

Here is a simplified view of my code (full working example here: https://gist.github.com/4329926):

class User(Base):

    __tablename__ = 'users_test_transaction'

 

    id = Column(Integer, primary_key=True)

    name = Column(Unicode(200))

    password = Column(Unicode(200))

 

    def delete_password(self):

        self.password = ""

        # Not sure if this is right, but my code has a lot of commit in a

        # model's method. This prevents me from using begin_nested().

        Session.commit()

 

    def delete(self):

        self.delete_password()

        raise TypeError("Nothing")

        self.name = "Deleted"

        Session.commit()

 

 

# Fake view

def delete_user(user):

    # I would like this function to happen in a transaction, so that if

    # any exception is raised in delete, it rolls back everything. Problem is,

    # user.delete() might directly or inderectly call commit().

    try:

        user.delete()

    except:

        # Because there's a commit in delete_password, this rollback will do

        # nothing.

        Session.rollback()

So the problem here is that the rollback does nothing, because there's a commit in the delete_password method (in this example there's only method called, in reality I have multiple methods). I think having stuff being committed in a model's method is antipattern (is it?) but I'd like to know if there's a simple way to get the desired behavior. I tried:

1. Manually setting a savepoint (Session.execute("SAVEPOINT soft_delete")). But commit in delete_password closes the connection so the savepoint is lost.
2. Using begin_nested and begin(subtransactions=True), but commit in delete_password closes the context.
3. I thought of using autocommit=True but it seems it's not recommended (and I'm not sure to understand all the implications of this), or creating an external transaction but the example in the doc is only for tests so I'm not sure if it's antipattern too. Also, getting access to the engine would require quite a small refactor of my app.

Any idea? Should I get rid of the commit in the model's methods (or add a do_not_commit arg to the function)? I wish there were a simple solution that do not require changing the model methods.

Thanks,

Charles

[Michael Bayer]

On Dec 19, 2012, at 4:29 AM, charlax wrote:

So the problem here is that the rollback does nothing, because there's a commit in the delete_password method (in this example there's only method called, in reality I have multiple methods). I think having stuff being committed in a model's method is antipattern (is it?)

I think it is.  I wish I could find more authoritative sources for this but in my opinion, as well as all the experience I've had with transaction-aware frameworks, transaction demarcation is specified in the application in exactly one place, in a position that surrounds all the business logic.  The business logic itself should not have any awareness of transaction demarcation.

I've updated the docs significantly to talk about this pattern here: http://docs.sqlalchemy.org/en/rel_0_8/orm/session.html#session-frequently-asked-questions, in the question "when do I construct a Session?".    Web frameworks present the simplest pattern, but if your application is not a web application, you'd want to decide on where your app's concept of "business methods" can be enclosed.

but I'd like to know if there's a simple way to get the desired behavior. I tried:

1. Manually setting a savepoint (Session.execute("SAVEPOINT soft_delete")). But commit in delete_password closes the connection so the savepoint is lost.
2. Using begin_nested and begin(subtransactions=True), but commit in delete_password closes the context

3. I thought of using autocommit=True but it seems it's not recommended (and I'm not sure to understand all the implications of this), or creating an external transaction but the example in the doc is only for tests so I'm not sure if it's antipattern too. Also, getting access to the engine would require quite a small refactor of my app.

the subtransaction thing could allow this, if your ad-hoc commit() method also called begin(subtransactions=True) before it did its work, but this implies the Session is being used in autocommit mode.

But there's no reason to use autocommit, while it used to be the default system of use years ago, now it's only there for certain framework integrations that want to be able to emit begin() ahead of time.  The Session otherwise shouldn't be used to emit SQL without a transaction present.

Which means, there's really no reason in the world you need to call commit() inside that method, *unless* you're trying to expose that particular bit of data to the outside world before the enclosing transaction is complete - and for that use case I'd use a different transaction altogether.   Otherwise, you're already in a transaction that isn't yet committed, which your app structure should ensure happens before the larger series of business methods is complete.

[Charles-Axel Dein]

Thanks Michael. Always spot on. And I don't think I can find a more authoritative source ;)

--
You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To post to this group, send email to sqlal...@googlegroups.com.
To unsubscribe from this group, send email to sqlalchemy+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/sqlalchemy?hl=en.