[MERGE] add lock hooks
Martin Pool
mbp at canonical.com
Fri Apr 4 05:33:28 BST 2008
Martin Pool has voted tweak.
Status is now: Conditionally approved
Comment:
This is really nice, far better than doing it from gc. I do have a few
irritations with the current structure which I can't help scratching
though.
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/
Shouldn't this be a private class?
+
+# 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.)
+
+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?
For details, see:
http://bundlebuggy.aaronbentley.com/request/%3C1207281650.5960.157.camel%40lifeless-64%3E
More information about the bazaar
mailing list