[MERGE] add lock hooks

Robert Collins robertc at robertcollins.net
Mon Apr 7 21:50:34 BST 2008


On Fri, 2008-04-04 at 00:33 -0400, Martin Pool wrote:
> 
> The name and the docstring implies these are active for all physical 
> locks, but they are only implemented for LockDirs, and we do
> currently 
> also use OS locks.  I think the hook name is probably best kept
> generic, 
> but you should at minimum document this in the dev guide, as well as
> in 
> news.
> 
> It's unfortunate there's no namespace allowing you to reach all hooks.
> 
> +class PhysicalLockHooks(Hooks):
> +    """hooks for physical lock activity."""
> 
> s//Hooks/

Do you mean 'Use a capital letter?' 

> Shouldn't this be a private class?

bzrlib.branch.BranchHooks isn't named with an _ either; one of the
benefits of the current hook layout is that these classes are public :P.

> +
> +# The hooks instance clients should register against. Do *NOT*
> import 
> this
> +# symbol, always reference it through lock.hooks.
> +hooks = PhysicalLockHooks()
> 
> It's unfortunate the current standard is an API that can so easily be 
> used incorrectly.  (But, again, not in scope for this particular
> patch.)

Its harder to use incorrectly in other modules because there is a top
level class like Branch that it can be an attribute of.

> +
> +class LockResult(object):
> 
> Docstring?



> +
> +    def record_hook(self, result):
> +        self._calls.append(result)
> +
> +    def reset_hooks(self):
> +        self._old_hooks = lock.hooks
> +        self.addCleanup(self.restore_hooks)
> +        lock.hooks = lock.PhysicalLockHooks()
> +
> 
> How about putting these on a separate class and then you can factor
> the common setup into the setUp method?

Hmm, I need other logic from the current class too though; I think I can
put the self._calls = [] line in reset_hooks though.

-Rob


-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080408/e5973005/attachment.pgp 


More information about the bazaar mailing list