Ok, so I try my hand at that decorator implementation, if that is the preferred way.
Regards Jan Schukat Christian Boos <[email protected]> schrieb am 27.07.2009 14:50:06: > > 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 -~----------~----~----~----~------~----~------~--~---
