[Checkins] SVN: transaction/trunk/ Merge chrism-fix.nonretryablecommitexceptions branch via

[Chris McDonough]

Log message for revision 127106:
  Merge chrism-fix.nonretryablecommitexceptions branch via
  
  svn merge -r127104:127105 svn+ssh://svn.zope.org/repos/main/transaction/branches/chrism-fix.nonretryablecommitexceptions
  
  

Changed:
  _U  transaction/trunk/
  U   transaction/trunk/CHANGES.txt
  _U  transaction/trunk/transaction/
  U   transaction/trunk/transaction/_manager.py
  U   transaction/trunk/transaction/tests/test_attempt.py

-=-

Property changes on: transaction/trunk
___________________________________________________________________
Modified: svn:ignore
   - *.egg
*.egg-info
build
dist
bin
parts
eggs
develop-eggs
.installed.cfg
.coverage

   + *.egg
*.egg-info
build
dist
bin
parts
eggs
develop-eggs
.installed.cfg
.coverage
.tox
nosetests.xml


Modified: transaction/trunk/CHANGES.txt
===================================================================
--- transaction/trunk/CHANGES.txt	2012-06-26 22:06:43 UTC (rev 127105)
+++ transaction/trunk/CHANGES.txt	2012-06-26 22:25:43 UTC (rev 127106)
@@ -4,7 +4,11 @@
 1.3.1 (unreleased)
 ------------------
 
-- TBD
+- When a non-retryable exception was raised as the result of a call to
+  ``transaction.manager.commit`` within the "attempts" machinery, the
+  exception was not reraised properly.  Symptom: an unrecoverable exception
+  such as ``Unsupported: Storing blobs in <somestorage> is not supported.``
+  would be swallowed inappropriately.
 
 1.3.0 (2012-05-16)
 ------------------


Property changes on: transaction/trunk/transaction
___________________________________________________________________
Modified: svn:ignore
   - *.so

   + *.so
coverage.xml


Modified: transaction/trunk/transaction/_manager.py
===================================================================
--- transaction/trunk/transaction/_manager.py	2012-06-26 22:06:43 UTC (rev 127105)
+++ transaction/trunk/transaction/_manager.py	2012-06-26 22:25:43 UTC (rev 127106)
@@ -16,6 +16,7 @@
 It coordinates application code and resource managers, so that they
 are associated with the right transaction.
 """
+import sys
 import threading
 
 from zope.interface import implementer
@@ -24,6 +25,7 @@
 from transaction._transaction import Transaction
 from transaction.interfaces import ITransactionManager
 from transaction.interfaces import TransientError
+from transaction.compat import reraise
 
 
 # We have to remember sets of synch objects, especially Connections.
@@ -152,6 +154,13 @@
     def __init__(self, manager):
         self.manager = manager
 
+    def _retry_or_raise(self, t, v, tb):
+        retry = self.manager._retryable(t, v)
+        self.manager.abort()
+        if retry:
+            return retry # suppress the exception if necessary
+        reraise(t, v, tb) # otherwise reraise the exception
+        
     def __enter__(self):
         return self.manager.__enter__()
 
@@ -159,13 +168,8 @@
         if v is None:
             try:
                 self.manager.commit()
-            except TransientError:
-                self.manager.abort()
-                return True # swallow
             except:
-                self.manager.abort()
-                return False # don't swallow
+                return self._retry_or_raise(*sys.exc_info())
         else:
-            retry = self.manager._retryable(t, v)
-            self.manager.abort()
-            return retry # swallow exception if True, else don't swallow
+            return self._retry_or_raise(t, v, tb)
+        

Modified: transaction/trunk/transaction/tests/test_attempt.py
===================================================================
--- transaction/trunk/transaction/tests/test_attempt.py	2012-06-26 22:06:43 UTC (rev 127105)
+++ transaction/trunk/transaction/tests/test_attempt.py	2012-06-26 22:25:43 UTC (rev 127106)
@@ -21,8 +21,9 @@
     def test___exit__no_exc_nonretryable_commit_exception(self):
         manager = DummyManager(raise_on_commit=ValueError)
         inst = self._makeOne(manager)
-        result = inst.__exit__(None, None, None)
-        self.assertFalse(result)
+        self.assertRaises(ValueError, inst.__exit__, None, None, None)
+        self.assertTrue(manager.committed)
+        self.assertTrue(manager.aborted)
 
     def test___exit__no_exc_abort_exception_after_nonretryable_commit_exc(self):
         manager = DummyManager(raise_on_abort=ValueError, 
@@ -53,8 +54,7 @@
     def test___exit__with_exception_value_nonretryable(self):
         manager = DummyManager()
         inst = self._makeOne(manager)
-        result = inst.__exit__(KeyError, KeyError(), None)
-        self.assertFalse(result)
+        self.assertRaises(KeyError, inst.__exit__, KeyError, KeyError(), None)
         self.assertFalse(manager.committed)
         self.assertTrue(manager.aborted)