[MERGE][#248153] Add autopack RPC

[Andrew Bennetts]

Martin Pool wrote:
> Martin Pool has voted comment.
> Status is now: Semi-approved
> Comment:
> This definitely deserves a mention in NEWS, including some kind of
> quantification of what improvement it will give you, and a description  
> of
> what will happen if the server is too old.

Currently it just falls back.  No warning is issued, although perhaps it
should.

I've added a NEWS entry.

[...]
> While we're here, it seems like an obvious opportunity to have the
> PackRepository.autopack command return the pack names data, so that we
> don't need to do another roundtrip.  I can see two counterarguments
> though, one being that it might not be trivial to inject it into the
> _real_repository, and the other that if we can often get away without
> vfs-based operations it may be unnecessary.  But at least the second  
> does
> not seem to be true in the near future.

John Arbash Meinel wrote:
] I agree that returning 'pack-names' seems reasonable, and it shouldn't
] be very hard to factor 'reload_pack_names' one more time to allow
] passing in the name list.

Fair enough.  It's more than just a list of names, but I've extended the
code to do this.

> +    elif err.error_verb == 'NotPackRepository':
> +        # There's no specific exception to raise for this, because it  
> shouldn't
> +        # happen unless something has changed the remote repo's format  
> between
> +        # smart requests.
> +        raise errors.InternalBzrError(
> +            'Tried to use PackRepository verb, but remote side says '
> +            '%s is not a pack repository.' % (find('repository')))
>      raise errors.UnknownErrorFromSmartServer(err)
>
> This seems a bit specific.  Should we just have a generic error for all
> rpcs meaning "this operation is not supported on this repository".

Hmm, ok.  I'm happy to do that, although I haven't made that change just
yet.

In the meantime, here's the current patch.  It adds a NEWS entry and
removes one more round-trip by returning the pack-names data as the
result of the autopack verb.

The update also fixes a shallow last minute boo boo I made in the final
revision of the previous bundle, which trivially broke it with a
traceback.  And it updates the RemoteRepository.autopack method to use
the new self._call helper that recently landed in bzr.dev.

The other thing that would be nice to improve is the duplication in the
two InterRepo _pack methods, although it's slightly harder to see how to
slice that nicely now that I've fixed that boo boo.

-Andrew.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: autopack-rpc-2.diff
Type: text/x-diff
Size: 47599 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20081031/220575ed/attachment-0001.bin