[MERGE] Handle lock contention on NFS

[Robert Collins]

On Mon, 2006-04-03 at 16:56 +1000, Martin Pool wrote:
> 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

Thanks.

> > 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.

I do agree with that.

> > +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.


Sure, thanks for the clearer text :).

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/20060404/561ca06d/attachment.pgp