[MERGE] Handle lock contention on NFS

[Martin Pool]

On  3 Apr 2006, Robert Collins <robertc at robertcollins.net> wrote:
> On Wed, 2006-03-29 at 10:05 -0500, Aaron Bentley wrote:
> 
> 
> > > 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.
> > 
> > Yes, that would be interesting.  Also, it would be nice to make it a
> > subclass of LocalTransport if that's possible.
> 
> Here is a decorator version of FakeNFS. This will by default backend
> onto a LocalRelpathServer, so it has actual disk backing.

+1 with some comments

> However, it does not appear as a 'LocalTransport' to our code that
> checks for instances of LocalTransport.
> 
> I realise this patch looks a lot bigger, but its 90% just moving the
> infrastructure the ReadOnlyTransport decorator used into a common base
> class to allow trivial decorators to be written.
> 
> I think that for the LocalHost checks it might be nice to add the
> following to Transport:
> 
>     def get_local_path(self):
>         """Returns a local path representation of this transports url.
> 
>         If this URL cannot be represented as a local path, NotLocalURL
>         will be raised.
> 
>         The default implementation raises NotLocalURL as we expect that
>         to be the common case.
>         """
>         raise NotLocalUrl(self.base)
> 
> Then the FakeNFSTransportDecorator can just do:
>         return self._decorated.get_local_path()
> 
> And it also solves the confusion we've had about whether local path urls
> are utf8 encoded or url encoded etc etc by giving them a specific
> protocol for getting a local path representation.
> 
> What do you think?

That sounds good.

Perhaps it should take a path parameter, and return the local path for
that file, or the root if no path is given.

I might change the doc to something like:

  If this transport accesses local files, return the local path to a
  file.

  If the files are not in the local filesystem (e.g. on another server,
  in memory, etc) then this raises NotLocalUrl. 

  This always returns a unicode string.

If you agree unicode would be best.

> +class TransportDecorator(Transport):
> +    """A no-change decorator for Transports.
> +    
> +    This does not use __getattr__ hacks as we need to ensure that
> +    new methods are overridden correctly.
> +
> +    This decorator class is not directly usable: subclasses must
> +    override the _get_url_prefix() class method to return the url prefix they 
> +    have chosen.
> +    """

The docstring needs to explain more about what the class is *for*, e.g.

    Subclasses of this are new transports that are based on an
    underlying transport and can override or intercept some 
    behavior.  For example ReadonlyTransportDecorator prevents 
    all write attempts, and FakeNFSTransportDecorator simulates 
    some NFS quirks.

It's arguably not ideal to refer to a subclass to explain the purpose of
a superclass, but I think an example goes a long way in explaining
abstractions.

-- 
Martin