Re: [Zope-dev] [ZODB-Dev] transaction as context manager, exception during commit

2011-02-24 Thread Chris Withers
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
___
Zope-Dev maillist  -  Zope-Dev@zope.org
https://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 https://mail.zope.org/mailman/listinfo/zope-announce
 https://mail.zope.org/mailman/listinfo/zope )


Re: [Zope-dev] [ZODB-Dev] transaction as context manager, exception during commit

2011-02-24 Thread Laurence Rowe
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
___
Zope-Dev maillist  -  Zope-Dev@zope.org
https://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 https://mail.zope.org/mailman/listinfo/zope-announce
 https://mail.zope.org/mailman/listinfo/zope )