[MERGE] Add helper class versionedfile.VersionedFilesDecorator for use with RemoteRepository in future.

[Robert Collins]

On Tue, 2009-02-24 at 09:08 -0600, John Arbash Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Robert Collins wrote:
> > As NEWS says, a helpful generic VersionedFilesDecorator; Andrew and I
> > intend to use this to avoid calls to 'start_commit_group' on
> > RemoteRepository having to call _ensure_real until actual operations are
> > undertaken.
> > 
> > -Rob
> > 
> > 
> 
> So IIRC, the idea is that you'll create one of this and *not* have
> populated the _backing_vf yet. And then once a call is made, to
> "repository.texts" (for example) it will call _ensure_real and then
> populate _backing_vf and respond.

Yup, precisely. Also we want to be able to trap add* and insert* methods
and use them to call repo._real_repository.start_write_group() if
needed, rather than always making a real repo when a write group is
started. This was for fetch, and we solved it another way so it can
actually wait; but we will want one of these soon.

> I'm a bit curious why you pass all the parameters to _hook_call, rather
> than just alerting it that it is being called?

So that RecordingVersionedFileDecorator, used in tests, was able to
subclass this rather than being totally separate.

> Also, why not use the __getattr__ pattern rather than having to proxy
> each function?

I find __getattr__ less clear; perhaps personal preference.

> If you are doing it this way, is there a way to hook it into the generic
> tests, so that we are sure if we add a new VF api, that it also gets
> implemented here?

It is hooked in - the interface tests run against one of these with the
hooking call set to just populate the _backing_vf.

> I'm a little concerned about the overhead of calling _hook_call for
> every action, though the only one we do a lot is get_parent_map. And
> then it should be hundreds and not thousands (though on Emacs, it could
> potentially be 80k or so). Probably it is ok.

Yup, be bad in inner loops - but an inner loop on a RemoteRepository
object is an inner loop with network latency in it - something that is
far worse.

> +class RecordingVersionedFilesDecorator(VersionedFilesDecorator):
>      """A minimal versioned files that records calls made on it.
>  ...
> 
> Instead of passing a function to VFD.__init__ you create an explicit
> 'def _hook_call' function. I think it would be better as:
> 
> def _rvf_hook_call(self):
>   ...
> 
> def __init__(self):
>   VFD.__init__(self, hook_call=self._rvf_hook_call)
> 
> Or some permutation on that.
> 
> I realize what you do *works*, but only as long as you don't call
> super().__init__, which doesn't seem like a good policy to require. At a
> minimum it needs to be documented in __init__. I would prefer if you
> passed the hook_call parameter.

I don't understand this; certainly in bzr we don't use super() anymore
anyhow, but even if we did, what I did works, doesn't it?

-Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090225/7355ee43/attachment.pgp