Merge lp://staging/~jelmer/bzr/hpss-_get-checkout-format into lp://staging/bzr

Proposed by Jelmer Vernooij
Status: Merged
Approved by: Martin Packman
Approved revision: no longer in the source branch.
Merged at revision: 6371
Proposed branch: lp://staging/~jelmer/bzr/hpss-_get-checkout-format
Merge into: lp://staging/bzr
Prerequisite: lp://staging/~jelmer/bzr/hpss-no-vfs
Diff against target: 313 lines (+159/-21)
9 files modified
bzrlib/controldir.py (+6/-1)
bzrlib/remote.py (+42/-11)
bzrlib/smart/bzrdir.py (+35/-0)
bzrlib/smart/request.py (+3/-0)
bzrlib/tests/blackbox/test_checkout.py (+4/-5)
bzrlib/tests/per_branch/test_create_checkout.py (+4/-4)
bzrlib/tests/test_remote.py (+37/-0)
bzrlib/tests/test_smart.py (+20/-0)
doc/en/release-notes/bzr-2.5.txt (+8/-0)
To merge this branch: bzr merge lp://staging/~jelmer/bzr/hpss-_get-checkout-format
Reviewer Review Type Date Requested Status
Martin Packman (community) Approve
Review via email: mp+85652@code.staging.launchpad.net

This proposal supersedes a proposal from 2011-12-13.

Commit message

Add HPSS call for ``BzrDir.checkout_metadir``.

Description of the change

Add HPSS call for BzrDir.checkout_metadir.

To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote : Posted in a previous version of this proposal

Does this actually make sense to ask across the network? Is this
something we could instead either get at the time the Branch is
opened, or just know from local data about the branch format?

Revision history for this message
Jelmer Vernooij (jelmer) wrote : Posted in a previous version of this proposal

On 11/29/2011 08:28 PM, Martin Pool wrote:
> Does this actually make sense to ask across the network? Is this
> something we could instead either get at the time the Branch is
> opened, or just know from local data about the branch format?
The checkout format can be different from that of the source branch in
some cases. We already have a BzrDir.cloning_metadir() HPSS method,
which behaves similarly, but is used for clone-oriented-operations (vs
checkout operations).

BzrBranch4 uses a metadir-based checkout format while it itself isn't
metadir-based, and SVN has a different checkout dir format than its
storage dir format. These probably aren't very relevant in practice, though.

Perhaps the HPSS call should be for BzrDir.checkout_metadir() rather
than Branch._get_checkout_format() though?

Cheers,

Jelmer

Revision history for this message
Martin Pool (mbp) wrote : Posted in a previous version of this proposal

It's nice to see the counts go further down.

I have a couple of concerns about this patch and others like it
 * there's no real documentation of what the method does; because this
is an interface where old bzr code talks to newer code it seems useful
to be a bit more explicit
 * I do wonder if we need a separate roundtrip to get stuff that it
seems could reasonably be fetched when the object is opened.

--
Martin

Revision history for this message
Jelmer Vernooij (jelmer) wrote : Posted in a previous version of this proposal

Am 14/12/11 00:37, schrieb Martin Pool:
> It's nice to see the counts go further down.
>
> I have a couple of concerns about this patch and others like it
> * there's no real documentation of what the method does; because this
> is an interface where old bzr code talks to newer code it seems useful
> to be a bit more explicit
That makes sense, I'll have a look at adding more documentation.

> * I do wonder if we need a separate roundtrip to get stuff that it
> seems could reasonably be fetched when the object is opened.
There is a similar pattern for getting the appropriate metadir format
for branch cloning, where we call the "BzrDir.cloning_metadir" HPSS verb
to figure out what the target format should be.

I agree that we shouldconsider chaining some of the HPSS requests.
Another case that comes to mind is to get the stacked-on branch URL
automatically when opening a branch, rather than having to request that
location using Branch.get_stacked_on_url.

I'm worried about taking too big leaps though - and adding a new branch
open call is more a lot more complex than BzrDir.checkout_metadir. The
addition of BzrDir.checkout_metadir will already save a couple of
roundtrips per checkout, so I would prefer to take that win now. Once we
get down to less than 15 roundtrips for a checkout, it should be a lot
easier to see which calls we can combine to eliminate even more
roundtrips. If that point happens to be before 2.5.0, we can always
remove BzrDir.checkout_metadir - existing 2.5 clients that try it will
automatically fall back to VFS if it's not there.

Cheers,

Jelmer

Revision history for this message
Martin Pool (mbp) wrote : Posted in a previous version of this proposal

works for me.

Revision history for this message
Martin Packman (gz) wrote :

Code changes and docstrings here look good, seems fine to land.

As discussed a little earlier, we do need some more general documentation for these HPSS changes once the various remaining branches have landed. Some notes for developers would be good, though it's true the smart protocol doesn't have an existing guide it would be obvious to add to. For users, we want an overview of which operations (remote lightweight checkout) are now how much better in what circumstances (2.5 for server and client).

review: Approve
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

sent to pqm by email

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

sent to pqm by email

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.