RFC: Opening/initializing locked objects

[Martin Pool]

On  9 Jan 2009, John Arbash Meinel <john at arbash-meinel.com> wrote:
> In looking at some of the traces of remote connections, you can see some
> inefficiencies in how we are handling things like locking the remote object.
> 
> For example, when creating a new Branch, we actually create the .bzr
> dir, lock it for writing, upload the content, then unlock it, and then
> start looking around for a Branch to be present in that directory.

I think that would be a great idea; it's obviously inefficient to return
a new-object unlocked since in every plausible case the caller will want
to actually put something into it.

> 
> I also know that we tend to lock an object as we create it, but then we
> return it unlocked to the caller, who then locks it as the first action.
> 
> It also would be nice to have a single RPC that could open whatever
> branch is present, along with its associated repository, and have
> everything primed for writing. So I'm proposing to add a new API, which
> is similar to our existing helpers, but does a bit more to combine
> things together.
> 
> I was thinking to call it Branch.open_locked() with an api of something
> like:
> 
> def open_locked(self, url, possible_transports=None, containing=False,
> 		writeable=False):
> 
> 
> The "containing" parameter would mean the return value would need to
> have a way to give the tail of the URL (as open_containing does now).
> I'm thinking to not change the return signature, but just allow the
> relpath to always be '' when containing is False.
> 
> Instead of 'writeable=False' we could do 'readonly=True', but I felt
> that 'readonly=False' wouldn't be as good of an indication that we want
> to write as 'writeable=True'.

I wonder if this should take a url or a Transport.  It's another case
where there is some minor proliferation of convenience interfaces.
For interactive ipython use giving a url is easier.

Indicating at this early stage whether we intend to write to it or not
might help give some clearer messages e.g. when pushing to Launchpad
(where we can only write if we know their username) or attempting to
push to http.

> I was also thinking about the "initialize" functions we have elsewhere.
> I don't think we can simply change them to return locked objects,
> because of API compatibility concerns. 'create_locked()' would be a good
> match for 'open_locked()' and flows a bit better than 'initialize_locked()'.
> 
> Alternatively we could add a "create" parameter to 'open_locked', but I
> think it needs to describe a format, which is turning it into a bit too
> overloaded, IMO.

However, it is a reasonably common case to want to push to a remote
location and create the branch if it doesn't exist, and we might like to
turn that into a single rpc.  However, we don't need to do it all in one
go; just make a patch that does the locking if you like and we'll be one
step closer.  

> 
> It would also be a design goal to make functions like
> "RepositoryAcquisitionPolicy.acquire_repository()" actually return a
> locked object.
> 
> This would also end up as an RPC
> 
> I'm certainly interested in hearing any feedback people have, before I
> spend much time on it.

> 
> John
> =:->
> 
-- 
Martin      <http://launchpad.net/~mbp>