Merge tim-deprecate-subtxn branch.

Subtransactions are deprecated in 3.5, and will go away
in 3.7.

NEWS.txt

@@ -27,16 +27,16 @@ Savepoints
   marked a savepoint as invalid after its first use.  The implementation has
   been repaired, to match the docs.
 
-Subtransactions
----------------
+Subtransactions are deprecated
+------------------------------
 
-- (3.5a4) Internal uses of subtransactions (transaction ``commit()`` or
-  ``abort()`` passing a true argument) were rewritten to use savepoints
-  instead.  Application code is strongly encouraged to do this too:
-  subtransactions are weaker, will be deprecated soon, and do not mix well
-  with savepoints (when you do a subtransaction commit, all current
-  savepoints are made unusable).  In general, a subtransaction commit
-  done just to free memory can be changed from::
+- (3.5a4) Subtransactions are deprecated, and will be removed in ZODB 3.7.
+  Use savepoints instead.  Savepoints are more powerful, and code using
+  subtransactions does not mix well with code using savepoints (a
+  subtransaction commit forces all current savepoints to become unusable, so
+  code using subtransactions can hurt newer code trying to use savepoints).
+  In general, a subtransaction commit done just to free memory can be changed
+  from::
 
       transaction.commit(1)
 
@@ -62,6 +62,10 @@ Subtransactions
 
       sp.rollback()
 
+- (3.5a4) Internal uses of subtransactions (transaction ``commit()`` or
+  ``abort()`` passing a true argument) were rewritten to use savepoints
+  instead.
+
 Multi-database
 --------------
 

src/ZODB/tests/testConnection.py

@@ -24,6 +24,12 @@ from ZODB.utils import p64, u64
 from ZODB.tests.warnhook import WarningsHook
 from zope.interface.verify import verifyObject
 
+# deprecated37  remove when subtransactions go away
+# Don't complain about subtxns in these tests.
+warnings.filterwarnings("ignore",
+                        ".*\nsubtransactions are deprecated",
+                        DeprecationWarning, __name__)
+
 class ConnectionDotAdd(unittest.TestCase):
 
     def setUp(self):

src/ZODB/tests/testSubTransaction.py

@@ -11,10 +11,16 @@
 # FOR A PARTICULAR PURPOSE.
 #
 ##############################################################################
-r"""
+"""
 ZODB subtransaction tests
 =========================
 
+Subtransactions are deprecated.  First we install a hook, to verify that
+deprecation warnings are generated.
+
+>>> hook = WarningsHook()
+>>> hook.install()
+
 Subtransactions are provided by a generic transaction interface, but
 only supported by ZODB.  These tests verify that some of the important
 cases work as expected.
@@ -61,6 +67,19 @@ We'll make a series of modifications in subtransactions.
 >>> shadow_a.value, shadow_b.value
 ('a0', 'b0')
 
+The subtransaction commit should have generated a deprecation wng:
+
+>>> len(hook.warnings)
+1
+>>> message, category, filename, lineno = hook.warnings[0]
+>>> print message
+This will be removed in ZODB 3.7:
+subtransactions are deprecated; use transaction.savepoint() instead of \
+transaction.commit(1)
+>>> category.__name__
+'DeprecationWarning'
+>>> hook.clear()
+
 >>> a.value = "a2"
 >>> c.value = "c1"
 >>> transaction.commit(1)
@@ -100,6 +119,21 @@ database state as of the last sub-transaction commit.  There is
 >>> a.value, b.value, c.value
 ('a1', 'b1', 'c0')
 
+The subtxn abort should also have generated a deprecation warning:
+
+>>> len(hook.warnings)
+1
+>>> message, category, filename, lineno = hook.warnings[0]
+>>> print message
+This will be removed in ZODB 3.7:
+subtransactions are deprecated; use sp.rollback() instead of \
+transaction.abort(1), where `sp` is the corresponding savepoint \
+captured earlier
+>>> category.__name__
+'DeprecationWarning'
+>>> hook.clear()
+
+
 Multiple aborts have no extra effect.
 
 >>> transaction.abort(1)
@@ -131,8 +165,14 @@ database state as of the last sub-transaction commit.  There is
 >>> a.value, b.value, c.value
 ('a0', 'b0', 'c0')
 
+We have to uninstall the hook so that other warnings don't get lost.
+
+>>> len(hook.warnings)  # we don't expect we captured other warnings
+0
+>>> hook.uninstall()
 """
 
+from ZODB.tests.warnhook import WarningsHook
 from zope.testing import doctest
 
 def test_suite():

src/ZODB/utils.py

@@ -38,6 +38,7 @@ __all__ = ['z64',
            'WeakSet',
            'DEPRECATED_ARGUMENT',
            'deprecated36',
+           'deprecated37',
            'get_pickle_metadata',
           ]
 
@@ -57,6 +58,13 @@ def deprecated36(msg):
     warnings.warn("This will be removed in ZODB 3.6:\n%s" % msg,
                   DeprecationWarning, stacklevel=3)
 
+# Raise DeprecationWarning, noting that the deprecated thing will go
+# away in ZODB 3.7.  Point to the caller of our caller (i.e., at the
+# code using the deprecated thing).
+def deprecated37(msg):
+    warnings.warn("This will be removed in ZODB 3.7:\n%s" % msg,
+                  DeprecationWarning, stacklevel=3)
+
 z64 = '\0'*8
 
 assert sys.hexversion >= 0x02030000

src/transaction/_manager.py

@@ -21,6 +21,11 @@ import thread
 
 from transaction._transaction import Transaction
 
+# Used for deprecated arguments.  ZODB.utils.DEPRECATED_ARGUMENT is
+# too hard to use here, due to the convoluted import dance across
+# __init__.py files.
+_marker = object()
+
 # We have to remember sets of synch objects, especially Connections.
 # But we don't want mere registration with a transaction manager to
 # keep a synch object alive forever; in particular, it's common
@@ -80,11 +85,26 @@ class TransactionManager(object):
     def unregisterSynch(self, synch):
         self._synchs.remove(synch)
 
-    def commit(self, sub=False):
-        self.get().commit(sub)
-
-    def abort(self, sub=False):
-        self.get().abort(sub)
+    def commit(self, sub=_marker):
+        if sub is _marker:
+            sub = None
+        else:
+            from ZODB.utils import deprecated37
+            deprecated37("subtransactions are deprecated; use "
+                         "transaction.savepoint() instead of "
+                         "transaction.commit(1)")
+        return self.get().commit(sub, deprecation_wng=False)
+
+    def abort(self, sub=_marker):
+        if sub is _marker:
+            sub = None
+        else:
+            from ZODB.utils import deprecated37
+            deprecated37("subtransactions are deprecated; use "
+                         "sp.rollback() instead of "
+                         "transaction.abort(1), where `sp` is the "
+                         "corresponding savepoint captured earlier")
+        return self.get().abort(sub, deprecation_wng=False)
 
     def savepoint(self, optimistic=False):
         return self.get().savepoint(optimistic)

src/transaction/_transaction.py

@@ -358,7 +358,15 @@ class Transaction(object):
         # in which case it would do nothing besides uselessly free() this
         # transaction.
 
-    def commit(self, subtransaction=False):
+    def commit(self, subtransaction=_marker, deprecation_wng=True):
+        if subtransaction is _marker:
+            subtransaction = 0
+        elif deprecation_wng:
+            from ZODB.utils import deprecated37
+            deprecated37("subtransactions are deprecated; use "
+                         "transaction.savepoint() instead of "
+                         "transaction.commit(1)")
+
         if self._savepoint2index:
             self._invalidate_all_savepoints()
 
@@ -464,7 +472,16 @@ class Transaction(object):
                 self.log.error("Error in tpc_abort() on manager %s",
                                rm, exc_info=sys.exc_info())
 
-    def abort(self, subtransaction=False):
+    def abort(self, subtransaction=_marker, deprecation_wng=True):
+        if subtransaction is _marker:
+            subtransaction = 0
+        elif deprecation_wng:
+            from ZODB.utils import deprecated37
+            deprecated37("subtransactions are deprecated; use "
+                         "sp.rollback() instead of "
+                         "transaction.abort(1), where `sp` is the "
+                         "corresponding savepoint captured earlier")
+
         if subtransaction:
             # TODO deprecate subtransactions.
             if not self._subtransaction_savepoint:

src/transaction/tests/test_transaction.py

@@ -40,9 +40,17 @@ $Id$
 """
 
 import unittest
+import warnings
+
 import transaction
 from ZODB.utils import positive_id
 
+# deprecated37  remove when subtransactions go away
+# Don't complain about subtxns in these tests.
+warnings.filterwarnings("ignore",
+                        ".*\nsubtransactions are deprecated",
+                        DeprecationWarning, __name__)
+
 class TransactionTests(unittest.TestCase):
 
     def setUp(self):
@@ -212,7 +220,8 @@ class TransactionTests(unittest.TestCase):
 
         try:
             self.transaction_manager.commit()
-        except TestTxnException: pass
+        except TestTxnException:
+            pass
 
         assert self.nosub1._p_jar.ctpc_abort == 1
         assert self.sub1._p_jar.ctpc_abort == 1
@@ -444,10 +453,14 @@ def test_beforeCommitHook():
       >>> log
       []
 
-    The hook is only called for a full commit, not for subtransactions.
+    The hook is only called for a full commit, not for a savepoint or
+    subtransaction.
 
       >>> t = transaction.begin()
       >>> t.beforeCommitHook(hook, 'A', kw1='B')
+      >>> dummy = t.savepoint()
+      >>> log
+      []
       >>> t.commit(subtransaction=True)
       >>> log
       []