Repository: repoze/repoze.retry

Issue:
Use ``transaction.interfaces.TransientError`` rather than ``ZODB.POSException.ConflictError``
`transaction.interfaces.TransientError` indicates that a transactions should be retried.  It's a generalization of ConflictError provided so that tools like `repoze.retry` don't have to depend on ZODB.

I'm in the process of making ZEO `ClientDisconnected` errors transient errors so that short disconnection due to server restart don't necessarily result in client errors.

(Someone should have fixed `ZPublisher.Publish.Retry` to subclass `TransientError` a long time ago. :( )


Discussion:

Patch:
diff --git a/.travis.yml b/.travis.yml
index d9fd26a..11bc8dc 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -18,6 +18,8 @@ matrix:
       env: TOXENV=pypy
     - python: pypy3
       env: TOXENV=pypy3
+    - python: 3.8
+      env: TOXENV=w_deps
     - python: 3.8
       env: TOXENV=cover
 
diff --git a/CHANGES.rst b/CHANGES.rst
index febccc4..aacba50 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -10,6 +10,8 @@
 
 - Sleep for delay on retrying using the delay parameter.
 
+- Retry ``transaction.interfaces.TransientError`` (if importable) by default.
+
 1.4 (2016-06-03)
 ----------------
 
diff --git a/repoze/retry/__init__.py b/repoze/retry/__init__.py
index fb03934..11effe0 100644
--- a/repoze/retry/__init__.py
+++ b/repoze/retry/__init__.py
@@ -6,6 +6,13 @@ import traceback
 from io import BytesIO
 from time import sleep
 
+# Avoid hard dependency on transaction.
+try:
+    from transaction.interfaces import TransientError
+except ImportError:
+    class TransientError(Exception):
+        pass
+
 # Avoid hard dependency on ZODB.
 try:
     from ZODB.POSException import ConflictError
@@ -43,7 +50,7 @@ class Retry:
         self.tries = tries
 
         if retryable is None:
-            retryable = (ConflictError, RetryException,)
+            retryable = (TransientError, ConflictError, RetryException,)
 
         if not isinstance(retryable, (list, tuple)):
             retryable = [retryable]
diff --git a/tox.ini b/tox.ini
index bc506a9..0dbc936 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,11 +1,21 @@
 [tox]
 envlist = 
-    py27,pypy,py35,py36,py37,py38,pypy3,cover,docs
+    py27,pypy,py35,py36,py37,py38,pypy3,w_deps,cover,docs
 
 [testenv]
 commands = 
     python setup.py -q test -q
 
+[testenv:w_deps]
+basepython =
+    python3.8
+deps =
+    transaction
+    ZODB
+    Zope
+commands = 
+    python setup.py -q test -q
+
 [testenv:cover]
 basepython =
     python3.8


Test patch:
diff --git a/repoze/retry/tests.py b/repoze/retry/tests.py
index f9e3281..3166a4b 100644
--- a/repoze/retry/tests.py
+++ b/repoze/retry/tests.py
@@ -1,7 +1,35 @@
 import unittest
 
+try:
+    import transaction
+except ImportError:
+    _HAVE_TRANSACTION = False
+else:
+    _HAVE_TRANSACTION = True
+
+try:
+    import ZODB
+except ImportError:
+    _HAVE_ZODB = False
+else:
+    _HAVE_ZODB = True
+
+try:
+    import ZPublisher
+except ImportError:
+    _HAVE_ZPUBLISHER = False
+else:
+    _HAVE_ZPUBLISHER = True
+
+
 class CEBase:
 
+    def _getTransientError(self):
+        from repoze.retry import TransientError
+        return TransientError
+
+    TransientError = property(_getTransientError,)
+
     def _getConflictError(self):
         from repoze.retry import ConflictError
         return ConflictError
@@ -53,6 +81,30 @@ class RetryTests(unittest.TestCase, CEBase):
         env['wsgi.errors'] = StringIO()
         return env
 
+    @unittest.skipUnless(_HAVE_TRANSACTION, "Needs transaction")
+    def test_ctor_defaults_w_transaction(self):
+        from transaction.interfaces import TransientError
+        application = DummyApplication(conflicts=0)
+        retry = self._makeOne(application, tries=4)
+        if hasattr(retry, 'retryable'):
+            self.assertIn(TransientError, retry.retryable)
+
+    @unittest.skipUnless(_HAVE_ZODB, "Needs ZODB")
+    def test_ctor_defaults_w_zodb(self):
+        from ZODB.POSException import ConflictError
+        application = DummyApplication(conflicts=0)
+        retry = self._makeOne(application, tries=4)
+        if hasattr(retry, 'retryable'):
+            self.assertIn(ConflictError, retry.retryable)
+
+    @unittest.skipUnless(_HAVE_ZPUBLISHER, "Needs ZPublisher")
+    def test_ctor_defaults_w_zpublisher(self):
+        from ZPublisher.Publish import Retry as RetryException
+        application = DummyApplication(conflicts=0)
+        retry = self._makeOne(application, tries=4)
+        if hasattr(retry, 'retryable'):
+            self.assertIn(RetryException, retry.retryable)
+
     def test_retryable_is_not_sequence(self):
         application = DummyApplication(conflicts=1)
         retry = self._makeOne(application, tries=4,
@@ -367,8 +419,12 @@ class FactoryTests(unittest.TestCase, CEBase):
         self.assertEqual(middleware.tries, 3)
         self.assertEqual(middleware.tries, 3)
         self.assertEqual(middleware.log_after_try_count, 1)
-        self.assertEqual(middleware.retryable,
-                         (self.ConflictError, self.RetryException))
+        expected = [
+            self.TransientError,
+            self.ConflictError,
+            self.RetryException,
+        ]
+        self.assertEqual(middleware.retryable, tuple(expected))
 
     def test_make_retry_override_tries(self):
         from repoze.retry import make_retry #FUT
@@ -376,8 +432,12 @@ class FactoryTests(unittest.TestCase, CEBase):
         middleware = make_retry(app, {}, tries=4)
         self.assertTrue(middleware.application is app)
         self.assertEqual(middleware.tries, 4)
-        self.assertEqual(middleware.retryable,
-                         (self.ConflictError, self.RetryException))
+        expected = [
+            self.TransientError,
+            self.ConflictError,
+            self.RetryException,
+        ]
+        self.assertEqual(middleware.retryable, tuple(expected))
 
     def test_make_retry_override_tries_write_error(self):
         from repoze.retry import make_retry #FUT