[MERGE] Handle lock contention on NFS

Robert Collins robertc at robertcollins.net
Wed Mar 29 11:54:30 BST 2006


On Tue, 2006-03-28 at 16:39 -0500, Aaron Bentley wrote:

This is missing the blank line before the first method in the class:

class Foo:
   '''docstring'''

   def foo(self):
...

> +class FakeNFSTransport(MemoryTransport):
> +    """A transport that behaves like NFS, for testing"""
> +    def rename(self, rel_from, rel_to):
> +        try:
> +            MemoryTransport.rename(self, rel_from, rel_to)
> +        except FileExists, e:
> +            abs_to = self._abspath(rel_to)
> +            if abs_to in self._dirs:
> +                raise ResourceBusy(rel_to)
> +            else:
> +                raise

Have you considered making this a decorator rather than a subclass - a
trivial amount of currying would let you then apply this to
MemoryServer/SFTP/Local etc and would IMO simplify the changes to
support this - MemoryServer would not need to change at all.


>  class _MemoryLock(object):
>      """This makes a lock."""
>  
> @@ -258,14 +272,19 @@
>  class MemoryServer(Server):
>      """Server for the MemoryTransport for testing with."""
>  
> -    def setUp(self):
> +    def setUp(self, scheme=None, transport_class=None):
>          """See bzrlib.transport.Server.setUp."""
>          self._dirs = {}
>          self._files = {}
>          self._locks = {}
> -        self._scheme = "memory+%s:" % id(self)
> +        if scheme is not None:
> +            self._scheme = scheme
> +        else:            
> +            self._scheme = "memory+%s:" % id(self)
> +        if transport_class is None:
> +            transport_class = MemoryTransport
>          def memory_factory(url):
> -            result = MemoryTransport(url)
> +            result = transport_class(url)
>              result._dirs = self._dirs
>              result._files = self._files
>              result._locks = self._locks
> @@ -281,7 +300,13 @@
>          return self._scheme
>  

Missing leading blank line again here.

> +class FakeNFSServer(MemoryServer):
> +    """A fake NFS server to use with the fake transport"""
> +    def setUp(self):
> +        MemoryServer.setUp(self, "fnfs+%s:" % id(self),
> FakeNFSTransport)



+0 from me - I'm ok with it as is, but I think its making the
MemoryTransport/MemoryServer slightly less maintainable and that we can
do it more nicely.

If you are interested in making it a decorator but not completely sure
of what I mean, I'll be happy to do a variation on that part of the
patch that does this.

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: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060329/8fff9b9f/attachment.pgp 


More information about the bazaar mailing list