Damn web-mailer. Need to get rid of that. Which is part of the reason I'm contributing now. Need to move forward faster with my own infrastructure, so I can hold all my stuff there. So in the interest of progress, I'll make an implementation ;)
Regards Jan Schukat Christian Boos <[email protected]> schrieb am 27.07.2009 18:04:11: > > [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 -~----------~----~----~----~------~----~------~--~---
