Re: [ZODB-Dev] transaction as context manager, exception during commit
On 24/02/2011 11:38, Laurence Rowe wrote: >> Hi Jim, >> >> The current __exit__ for transaction managers looks like this: >> >> def __exit__(self, t, v, tb): >> if v is None: >> self.commit() >> else: >> self.abort() >> >> ..which means that if you're using the transaction package as a context >> manager and, say, a relational database integrity constraint is >> violated, then you're left with a hosed transaction that still needs >> aborting. >> >> How would you feel about the above changing to: >> >> def __exit__(self, t, v, tb): >> if v is None: >> try: >> self.commit() >> except: >> self.abort() >> raise >> else: >> self.abort() >> >> If this is okay, I'll be happy to write the tests and make the changes >> provided someone does a release when I have... > > Looking at the way ZPublisher handles this, I think you're right. Cool, I'll wait for Jim's verdict (and some time to implement it ;-)) before diving in... > I > think you might also need to modify the __exit__ in Attempt, which > additionally handles retrying transactions that fail. Yeah, I see a bug relating to this was reported yesterday: https://bugs.launchpad.net/bugs/724332 I know it's a different problem, but sounds like the code for attempt needs some love... cheers, Chris -- Simplistix - Content Management, Batch Processing & Python Consulting - http://www.simplistix.co.uk ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] transaction as context manager, exception during commit
On 24 February 2011 10:17, Chris Withers wrote: > Hi Jim, > > The current __exit__ for transaction managers looks like this: > > def __exit__(self, t, v, tb): > if v is None: > self.commit() > else: > self.abort() > > ..which means that if you're using the transaction package as a context > manager and, say, a relational database integrity constraint is > violated, then you're left with a hosed transaction that still needs > aborting. > > How would you feel about the above changing to: > > def __exit__(self, t, v, tb): > if v is None: > try: > self.commit() > except: > self.abort() > raise > else: > self.abort() > > If this is okay, I'll be happy to write the tests and make the changes > provided someone does a release when I have... Looking at the way ZPublisher handles this, I think you're right. I think you might also need to modify the __exit__ in Attempt, which additionally handles retrying transactions that fail. Laurence ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
[ZODB-Dev] transaction as context manager, exception during commit
Hi Jim, The current __exit__ for transaction managers looks like this: def __exit__(self, t, v, tb): if v is None: self.commit() else: self.abort() ..which means that if you're using the transaction package as a context manager and, say, a relational database integrity constraint is violated, then you're left with a hosed transaction that still needs aborting. How would you feel about the above changing to: def __exit__(self, t, v, tb): if v is None: try: self.commit() except: self.abort() raise else: self.abort() If this is okay, I'll be happy to write the tests and make the changes provided someone does a release when I have... cheers, Chris -- Simplistix - Content Management, Batch Processing & Python Consulting - http://www.simplistix.co.uk ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev