[Checkins] SVN: transaction/trunk/ merge chrism-fix-attempts branch to trunk

[Chris McDonough]

Log message for revision 124798:
  merge chrism-fix-attempts branch to trunk

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

-=-
Modified: transaction/trunk/CHANGES.txt
===================================================================
--- transaction/trunk/CHANGES.txt	2012-03-29 21:47:03 UTC (rev 124797)
+++ transaction/trunk/CHANGES.txt	2012-03-29 22:25:27 UTC (rev 124798)
@@ -6,6 +6,13 @@
 
 - Python 3.3 compatibility.
 
+- Fix "for attempt in transaction.attempts(x)" machinery, which would not
+  retry a transaction if its implicit call to ``.commit()`` itself raised a
+  transient error.  Symptom: seeing conflict errors even though you thought
+  you were retrying some number of times via the "attempts" machinery (the
+  first attempt to generate an exception during commit would cause that
+  exception to be raised).
+
 1.2.0 (2011-12-05)
 ------------------
 

Modified: transaction/trunk/transaction/_manager.py
===================================================================
--- transaction/trunk/transaction/_manager.py	2012-03-29 21:47:03 UTC (rev 124797)
+++ transaction/trunk/transaction/_manager.py	2012-03-29 22:25:27 UTC (rev 124798)
@@ -135,8 +135,15 @@
 
     def __exit__(self, t, v, tb):
         if v is None:
-            self.manager.commit()
+            try:
+                self.manager.commit()
+            except TransientError:
+                self.manager.abort()
+                return True # swallow
+            except:
+                self.manager.abort()
+                return False # don't swallow
         else:
             retry = self.manager._retryable(t, v)
             self.manager.abort()
-            return retry
+            return retry # swallow exception if True, else don't swallow

Copied: transaction/trunk/transaction/tests/test_attempt.py (from rev 124797, transaction/branches/chrism-fix-attempts/transaction/tests/test_attempt.py)
===================================================================
--- transaction/trunk/transaction/tests/test_attempt.py	                        (rev 0)
+++ transaction/trunk/transaction/tests/test_attempt.py	2012-03-29 22:25:27 UTC (rev 124798)
@@ -0,0 +1,85 @@
+import unittest
+
+class TestAttempt(unittest.TestCase):
+    def _makeOne(self, manager):
+        from transaction._manager import Attempt
+        return Attempt(manager)
+
+    def test___enter__(self):
+        manager = DummyManager()
+        inst = self._makeOne(manager)
+        inst.__enter__()
+        self.assertTrue(manager.entered)
+
+    def test___exit__no_exc_no_commit_exception(self):
+        manager = DummyManager()
+        inst = self._makeOne(manager)
+        result = inst.__exit__(None, None, None)
+        self.assertFalse(result)
+        self.assertTrue(manager.committed)
+
+    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)
+
+    def test___exit__no_exc_abort_exception_after_nonretryable_commit_exc(self):
+        manager = DummyManager(raise_on_abort=ValueError, 
+                               raise_on_commit=KeyError)
+        inst = self._makeOne(manager)
+        self.assertRaises(ValueError, inst.__exit__, None, None, None)
+        self.assertTrue(manager.committed)
+        self.assertTrue(manager.aborted)
+        
+    def test___exit__no_exc_retryable_commit_exception(self):
+        from transaction.interfaces import TransientError
+        manager = DummyManager(raise_on_commit=TransientError)
+        inst = self._makeOne(manager)
+        result = inst.__exit__(None, None, None)
+        self.assertTrue(result)
+        self.assertTrue(manager.committed)
+        self.assertTrue(manager.aborted)
+
+    def test___exit__with_exception_value_retryable(self):
+        from transaction.interfaces import TransientError
+        manager = DummyManager()
+        inst = self._makeOne(manager)
+        result = inst.__exit__(TransientError, TransientError(), None)
+        self.assertTrue(result)
+        self.assertFalse(manager.committed)
+        self.assertTrue(manager.aborted)
+
+    def test___exit__with_exception_value_nonretryable(self):
+        manager = DummyManager()
+        inst = self._makeOne(manager)
+        result = inst.__exit__(KeyError, KeyError(), None)
+        self.assertFalse(result)
+        self.assertFalse(manager.committed)
+        self.assertTrue(manager.aborted)
+        
+class DummyManager(object):
+    entered = False
+    committed = False
+    aborted = False
+    
+    def __init__(self, raise_on_commit=None, raise_on_abort=None):
+        self.raise_on_commit = raise_on_commit
+        self.raise_on_abort = raise_on_abort
+
+    def _retryable(self, t, v):
+        from transaction._manager import TransientError
+        return issubclass(t, TransientError)
+        
+    def __enter__(self):
+        self.entered = True
+
+    def abort(self):
+        self.aborted = True
+        if self.raise_on_abort:
+            raise self.raise_on_abort
+        
+    def commit(self):
+        self.committed = True
+        if self.raise_on_commit:
+            raise self.raise_on_commit