Code review comment for lp:~henninge/bzr/bug-522195-contextmanagers

Revision history for this message
Henning Eggers (henninge) wrote :

= Bug 522195 =

The current bzrlib is missing the ability to easily and safely lock branches, trees, and repositories using the new (well, since Python 2.5) 'with' statement. Using this construct to acquire locks on such objects makes sure they get unlocked again, even in case of an exception or a function returning within the statement. This all happens in one line with indention, making it very consise and easy to read.

This branch adds factories for context managers to Branch, Tree, and Repository objects. The factories are called 'read_locking' and 'write_locking' respectively. I chose these names, because
 - they read nicely in context of the 'with' statement because they are nouns.
 - they cannot be easily confused with 'lock_read', as would have been the case with a name like 'read_lock' (which would have been a noun, too.)

To use the context managers, simply do something like this:

  with lockable.read_locking():
      lockable.read_data()

After the with statement, the lockable is guranteed to be unlocked again, no matter what happens in its body.

I created Mixins in bzrlib.contextmanagers that were added to Branch, Tree, MutableTree, and Repository so that all subclasses of those now support this construct. Other classes in bzrlib might be lockable but I confined myself to these most important classes for now. Other classes can be extended easily using the Mixins.

I saw that some lock_write() methods take a "token" parameter although this does not seem to be the case for any of the classes mentioned above. Still, I added the ability for the write_locking() factory to pass any keyword argument into lock_write().

== Tests ==

As described in test_contextmanagers.py, compatibility with Python 2.4 makes the test of the context managers with an actual 'with' statement impossible. This test module will have to be updated once Python 2.5 is set as the lower limit.

I hope the test module is not too confusing with all the Mixins, Dummy class and with statement simulation code coming before the actual test cases.

Test command: ./bzr selftest -v contextmanagers

« Back to merge proposal