Remy Blank wrote:
> Christian Boos wrote:
>
>> There was some negative reactions at the time I proposed it for
>> integration into trunk, because some people didn't like the
>> `transaction` helper function I mixed in there (note in passing that by
>> using such a helper consistently for performing commit/rollback on SQL
>> transactions instead of relying on the gc for the eventual rollback, we
>> wouldn't have the occasional "current transaction is aborted" error).
>>
>
> FWIW, I rather liked the transaction() idea, but I would have
> implemented it as a function decorator, and used an inner function for
> the DB code (example from attachment.py):
>
> def delete(self, db=None):
> assert self.filename, 'Cannot delete non-existent attachment'
>
> @as_transaction(self.env, db)
> def execute(db):
> cursor = db.cursor()
> cursor.execute("DELETE FROM attachment WHERE type=%s "
> "AND id=%s AND filename=%s",
> (self.parent_realm, self.parent_id,
> self.filename))
> if os.path.isfile(self.path):
> try:
> os.unlink(self.path)
> except OSError, e:
> self.env.log.error('Failed to delete attachment '
> 'file %s: %s', self.path,
> exception_to_unicode(e, traceback=True))
> raise TracError(_('Could not delete attachment'))
>
> self.env.log.info('Attachment removed: %s' % self.title)
>
> for listener in AttachmentModule(self.env).change_listeners:
> listener.attachment_deleted(self)
>
> The as_transaction decorator would set up the database connection if
> necessary, call the decorated function and commit or rollback if necessary.
>
> This would also have the additional benefit that it becomes easy to move
> to context managers once we drop support for Python 2.4, by replacing:
>
> @as_transaction(self.env, db)
> def execute(db):
>
> with:
>
> with transaction(self.env, db) as db:
>
Even better!
Great, so I would suggest adopting the above pattern for 0.12.
It's just not a cosmetic thing, we have plenty of evidence now that
leaving connections in a non-rollbacked state after an error leads to
problems (I'm writing this while triaging
http://trac.edgewall.org/ticket/8379).
Other opinions?
-- Christian
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Trac
Development" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
