Rev 4732: (andrew) Add 'only_raises' decorator, in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Thu Oct 8 04:11:43 BST 2009
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 4732 [merge]
revision-id: pqm at pqm.ubuntu.com-20091008031135-6d7vxh4s0umav1eo
parent: pqm at pqm.ubuntu.com-20091006204548-bjnc3z4k256ppimz
parent: andrew.bennetts at canonical.com-20091008015533-pfzig4dpr3kqt2cc
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2009-10-08 04:11:35 +0100
message:
(andrew) Add 'only_raises' decorator,
use it to suppress most errors from Branch/Repository.unlock.
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/branch.py branch.py-20050309040759-e4baf4e0d046576e
bzrlib/decorators.py decorators.py-20060112082512-6bfc2d882df1698d
bzrlib/lockable_files.py control_files.py-20051111201905-bb88546e799d669f
bzrlib/lockdir.py lockdir.py-20060220222025-98258adf27fbdda3
bzrlib/remote.py remote.py-20060720103555-yeeg2x51vn0rbtdp-1
bzrlib/repofmt/pack_repo.py pack_repo.py-20070813041115-gjv5ma7ktfqwsjgn-1
bzrlib/repository.py rev_storage.py-20051111201905-119e9401e46257e3
bzrlib/tests/__init__.py selftest.py-20050531073622-8d0e3c8845c97a64
bzrlib/tests/lock_helpers.py LockHelpers.py-20060707151933-tav3o2hpibwi53u4-1
bzrlib/tests/per_branch/test_locking.py test_locking.py-20060707151933-tav3o2hpibwi53u4-4
bzrlib/tests/per_repository/test_write_group.py test_write_group.py-20070716105516-89n34xtogq5frn0m-1
bzrlib/tests/test_decorators.py test_decorators.py-20060113063037-0e7bd4566758f4fa
doc/developers/HACKING.txt HACKING-20050805200004-2a5dc975d870f78c
=== modified file 'NEWS'
--- a/NEWS 2009-10-06 20:45:48 +0000
+++ b/NEWS 2009-10-08 01:55:33 +0000
@@ -157,6 +157,13 @@
See also <https://answers.launchpad.net/bzr/+faq/703>.
(Martin Pool, #406113, #430529)
+* Secondary errors that occur during Branch.unlock and Repository.unlock
+ no longer obscure the original error. These methods now use a new
+ decorator, ``only_raises``. This fixes many causes of
+ ``TooManyConcurrentRequests`` and similar errors.
+ (Andrew Bennetts, #429747)
+
+
Documentation
*************
=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py 2009-08-19 18:04:49 +0000
+++ b/bzrlib/branch.py 2009-09-30 07:02:41 +0000
@@ -46,7 +46,7 @@
)
""")
-from bzrlib.decorators import needs_read_lock, needs_write_lock
+from bzrlib.decorators import needs_read_lock, needs_write_lock, only_raises
from bzrlib.hooks import HookPoint, Hooks
from bzrlib.inter import InterObject
from bzrlib import registry
@@ -2160,6 +2160,7 @@
self.repository.unlock()
raise
+ @only_raises(errors.LockNotHeld, errors.LockBroken)
def unlock(self):
try:
self.control_files.unlock()
=== modified file 'bzrlib/decorators.py'
--- a/bzrlib/decorators.py 2009-03-23 14:59:43 +0000
+++ b/bzrlib/decorators.py 2009-10-02 06:12:20 +0000
@@ -24,6 +24,8 @@
import sys
+from bzrlib import trace
+
def _get_parameters(func):
"""Recreate the parameters for a function using introspection.
@@ -204,6 +206,31 @@
return write_locked
+def only_raises(*errors):
+ """Make a decorator that will only allow the given error classes to be
+ raised. All other errors will be logged and then discarded.
+
+ Typical use is something like::
+
+ @only_raises(LockNotHeld, LockBroken)
+ def unlock(self):
+ # etc
+ """
+ def decorator(unbound):
+ def wrapped(*args, **kwargs):
+ try:
+ return unbound(*args, **kwargs)
+ except errors:
+ raise
+ except:
+ trace.mutter('Error suppressed by only_raises:')
+ trace.log_exception_quietly()
+ wrapped.__doc__ = unbound.__doc__
+ wrapped.__name__ = unbound.__name__
+ return wrapped
+ return decorator
+
+
# Default is more functionality, 'bzr' the commandline will request fast
# versions.
needs_read_lock = _pretty_needs_read_lock
=== modified file 'bzrlib/lockable_files.py'
--- a/bzrlib/lockable_files.py 2009-07-27 05:39:01 +0000
+++ b/bzrlib/lockable_files.py 2009-09-30 07:02:41 +0000
@@ -32,8 +32,7 @@
""")
from bzrlib.decorators import (
- needs_read_lock,
- needs_write_lock,
+ only_raises,
)
from bzrlib.symbol_versioning import (
deprecated_in,
@@ -221,6 +220,7 @@
"""Setup a write transaction."""
self._set_transaction(transactions.WriteTransaction())
+ @only_raises(errors.LockNotHeld, errors.LockBroken)
def unlock(self):
if not self._lock_mode:
return lock.cant_unlock_not_held(self)
=== modified file 'bzrlib/lockdir.py'
--- a/bzrlib/lockdir.py 2009-07-27 05:24:02 +0000
+++ b/bzrlib/lockdir.py 2009-09-30 07:02:41 +0000
@@ -112,6 +112,7 @@
lock,
)
import bzrlib.config
+from bzrlib.decorators import only_raises
from bzrlib.errors import (
DirectoryNotEmpty,
FileExists,
@@ -286,6 +287,7 @@
info_bytes)
return tmpname
+ @only_raises(LockNotHeld, LockBroken)
def unlock(self):
"""Release a held lock
"""
=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py 2009-10-02 05:43:41 +0000
+++ b/bzrlib/remote.py 2009-10-08 01:50:30 +0000
@@ -33,7 +33,7 @@
)
from bzrlib.branch import BranchReferenceFormat
from bzrlib.bzrdir import BzrDir, RemoteBzrDirFormat
-from bzrlib.decorators import needs_read_lock, needs_write_lock
+from bzrlib.decorators import needs_read_lock, needs_write_lock, only_raises
from bzrlib.errors import (
NoSuchRevision,
SmartProtocolError,
@@ -1082,6 +1082,7 @@
else:
raise errors.UnexpectedSmartServerResponse(response)
+ @only_raises(errors.LockNotHeld, errors.LockBroken)
def unlock(self):
if not self._lock_count:
return lock.cant_unlock_not_held(self)
@@ -2383,6 +2384,7 @@
return
raise errors.UnexpectedSmartServerResponse(response)
+ @only_raises(errors.LockNotHeld, errors.LockBroken)
def unlock(self):
try:
self._lock_count -= 1
=== modified file 'bzrlib/repofmt/pack_repo.py'
--- a/bzrlib/repofmt/pack_repo.py 2009-09-08 05:51:36 +0000
+++ b/bzrlib/repofmt/pack_repo.py 2009-09-30 07:02:41 +0000
@@ -54,7 +54,7 @@
revision as _mod_revision,
)
-from bzrlib.decorators import needs_write_lock
+from bzrlib.decorators import needs_write_lock, only_raises
from bzrlib.btree_index import (
BTreeGraphIndex,
BTreeBuilder,
@@ -2345,6 +2345,7 @@
packer = ReconcilePacker(collection, packs, extension, revs)
return packer.pack(pb)
+ @only_raises(errors.LockNotHeld, errors.LockBroken)
def unlock(self):
if self._write_lock_count == 1 and self._write_group is not None:
self.abort_write_group()
=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py 2009-09-24 04:54:19 +0000
+++ b/bzrlib/repository.py 2009-10-08 01:50:30 +0000
@@ -49,7 +49,7 @@
from bzrlib.testament import Testament
""")
-from bzrlib.decorators import needs_read_lock, needs_write_lock
+from bzrlib.decorators import needs_read_lock, needs_write_lock, only_raises
from bzrlib.inter import InterObject
from bzrlib.inventory import (
Inventory,
@@ -1720,6 +1720,7 @@
self.start_write_group()
return result
+ @only_raises(errors.LockNotHeld, errors.LockBroken)
def unlock(self):
if (self.control_files._lock_count == 1 and
self.control_files._lock_mode == 'w'):
=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py 2009-10-01 14:44:43 +0000
+++ b/bzrlib/tests/__init__.py 2009-10-08 01:50:30 +0000
@@ -1146,6 +1146,25 @@
self.fail("Incorrect length: wanted %d, got %d for %r" % (
length, len(obj_with_len), obj_with_len))
+ def assertLogsError(self, exception_class, func, *args, **kwargs):
+ """Assert that func(*args, **kwargs) quietly logs a specific exception.
+ """
+ from bzrlib import trace
+ captured = []
+ orig_log_exception_quietly = trace.log_exception_quietly
+ try:
+ def capture():
+ orig_log_exception_quietly()
+ captured.append(sys.exc_info())
+ trace.log_exception_quietly = capture
+ func(*args, **kwargs)
+ finally:
+ trace.log_exception_quietly = orig_log_exception_quietly
+ self.assertLength(1, captured)
+ err = captured[0][1]
+ self.assertIsInstance(err, exception_class)
+ return err
+
def assertPositive(self, val):
"""Assert that val is greater than 0."""
self.assertTrue(val > 0, 'expected a positive value, but got %s' % val)
=== modified file 'bzrlib/tests/lock_helpers.py'
--- a/bzrlib/tests/lock_helpers.py 2009-04-15 07:30:34 +0000
+++ b/bzrlib/tests/lock_helpers.py 2009-09-30 07:02:41 +0000
@@ -17,6 +17,7 @@
"""Helper functions/classes for testing locking"""
from bzrlib import errors
+from bzrlib.decorators import only_raises
class TestPreventLocking(errors.LockError):
@@ -68,6 +69,7 @@
return self._other.lock_write()
raise TestPreventLocking('lock_write disabled')
+ @only_raises(errors.LockNotHeld, errors.LockBroken)
def unlock(self):
self._sequence.append((self._other_id, 'ul', self._allow_unlock))
if self._allow_unlock:
=== modified file 'bzrlib/tests/per_branch/test_locking.py'
--- a/bzrlib/tests/per_branch/test_locking.py 2009-07-10 10:46:00 +0000
+++ b/bzrlib/tests/per_branch/test_locking.py 2009-09-30 07:02:41 +0000
@@ -139,7 +139,7 @@
try:
self.assertTrue(b.is_locked())
self.assertTrue(b.repository.is_locked())
- self.assertRaises(TestPreventLocking, b.unlock)
+ self.assertLogsError(TestPreventLocking, b.unlock)
if self.combined_control:
self.assertTrue(b.is_locked())
else:
@@ -183,7 +183,7 @@
try:
self.assertTrue(b.is_locked())
self.assertTrue(b.repository.is_locked())
- self.assertRaises(TestPreventLocking, b.unlock)
+ self.assertLogsError(TestPreventLocking, b.unlock)
self.assertTrue(b.is_locked())
self.assertTrue(b.repository.is_locked())
=== modified file 'bzrlib/tests/per_repository/test_write_group.py'
--- a/bzrlib/tests/per_repository/test_write_group.py 2009-09-08 06:25:26 +0000
+++ b/bzrlib/tests/per_repository/test_write_group.py 2009-09-30 07:02:41 +0000
@@ -84,7 +84,7 @@
# don't need a specific exception for now - this is
# really to be sure it's used right, not for signalling
# semantic information.
- self.assertRaises(errors.BzrError, repo.unlock)
+ self.assertLogsError(errors.BzrError, repo.unlock)
# after this error occurs, the repository is unlocked, and the write
# group is gone. you've had your chance, and you blew it. ;-)
self.assertFalse(repo.is_locked())
=== modified file 'bzrlib/tests/test_decorators.py'
--- a/bzrlib/tests/test_decorators.py 2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/test_decorators.py 2009-10-02 06:12:20 +0000
@@ -23,11 +23,15 @@
from bzrlib.tests import TestCase
-def create_decorator_sample(style, except_in_unlock=False):
+class SampleUnlockError(Exception):
+ pass
+
+
+def create_decorator_sample(style, unlock_error=None):
"""Create a DecoratorSample object, using specific lock operators.
:param style: The type of lock decorators to use (fast/pretty/None)
- :param except_in_unlock: If True, raise an exception during unlock
+ :param unlock_error: If specified, an error to raise from unlock.
:return: An instantiated DecoratorSample object.
"""
@@ -58,10 +62,11 @@
def lock_write(self):
self.actions.append('lock_write')
+ @decorators.only_raises(SampleUnlockError)
def unlock(self):
- if except_in_unlock:
+ if unlock_error:
self.actions.append('unlock_fail')
- raise KeyError('during unlock')
+ raise unlock_error
else:
self.actions.append('unlock')
@@ -119,28 +124,28 @@
def test_read_lock_raises_original_error(self):
sam = create_decorator_sample(self._decorator_style,
- except_in_unlock=True)
+ unlock_error=SampleUnlockError())
self.assertRaises(TypeError, sam.fail_during_read)
self.assertEqual(['lock_read', 'fail_during_read', 'unlock_fail'],
sam.actions)
def test_write_lock_raises_original_error(self):
sam = create_decorator_sample(self._decorator_style,
- except_in_unlock=True)
+ unlock_error=SampleUnlockError())
self.assertRaises(TypeError, sam.fail_during_write)
self.assertEqual(['lock_write', 'fail_during_write', 'unlock_fail'],
sam.actions)
def test_read_lock_raises_unlock_error(self):
sam = create_decorator_sample(self._decorator_style,
- except_in_unlock=True)
- self.assertRaises(KeyError, sam.frob)
+ unlock_error=SampleUnlockError())
+ self.assertRaises(SampleUnlockError, sam.frob)
self.assertEqual(['lock_read', 'frob', 'unlock_fail'], sam.actions)
def test_write_lock_raises_unlock_error(self):
sam = create_decorator_sample(self._decorator_style,
- except_in_unlock=True)
- self.assertRaises(KeyError, sam.bank, 'bar', biz='bing')
+ unlock_error=SampleUnlockError())
+ self.assertRaises(SampleUnlockError, sam.bank, 'bar', biz='bing')
self.assertEqual(['lock_write', ('bank', 'bar', 'bing'),
'unlock_fail'], sam.actions)
@@ -276,3 +281,21 @@
finally:
decorators.needs_read_lock = cur_read
decorators.needs_write_lock = cur_write
+
+
+class TestOnlyRaisesDecorator(TestCase):
+
+ def raise_ZeroDivisionError(self):
+ 1/0
+
+ def test_raises_approved_error(self):
+ decorator = decorators.only_raises(ZeroDivisionError)
+ decorated_meth = decorator(self.raise_ZeroDivisionError)
+ self.assertRaises(ZeroDivisionError, decorated_meth)
+
+ def test_quietly_logs_unapproved_errors(self):
+ decorator = decorators.only_raises(IOError)
+ decorated_meth = decorator(self.raise_ZeroDivisionError)
+ self.assertLogsError(ZeroDivisionError, decorated_meth)
+
+
=== modified file 'doc/developers/HACKING.txt'
--- a/doc/developers/HACKING.txt 2009-09-30 14:56:21 +0000
+++ b/doc/developers/HACKING.txt 2009-10-08 01:50:30 +0000
@@ -671,6 +671,19 @@
may not catch every case but it's still useful sometimes.
+Cleanup methods
+===============
+
+Often when something has failed later code, including cleanups invoked
+from ``finally`` blocks, will fail too. These secondary failures are
+generally uninteresting compared to the original exception. So use the
+``only_raises`` decorator (from ``bzrlib.decorators``) for methods that
+are typically called in ``finally`` blocks, such as ``unlock`` methods.
+For example, ``@only_raises(LockNotHeld, LockBroken)``. All errors that
+are unlikely to be a knock-on failure from an previous failure should be
+allowed.
+
+
Factories
=========
More information about the bazaar-commits
mailing list