[email protected] wrote: > Ok, so I try my hand at that decorator implementation, if that is the > preferred way. > >
Your mailer has definitely a problem with message threading ;-) See http://groups.google.com/group/trac-dev/browse_frm/thread/6f6fb445b07867d7 and look at how your messages appear in the tree... Besides, before investing too much time on this, you should wait a bit to see if we actually reach a consensus on this, unless you want to contribute a working "proof-of-concept", which would indeed be a good thing. -- Christian > 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 -~----------~----~----~----~------~----~------~--~---
