[PATCH] remote_branch with asyncore

Aaron Bentley aaron.bentley at utoronto.ca
Mon Jun 27 22:37:25 BST 2005


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> On Mon, 2005-06-27 at 09:58 -0400, Aaron Bentley wrote:
> 
>      def copy_multi_helper(self, other, to_copy, pb):
> -        from store import default_copy_multi
> +        from store import default_copy_multi, ImmutableStore
> +        if isinstance(other, ImmutableStore):
> +            return self.copy_multi_helper_parallel(other, to_copy, pb)
>          return default_copy_multi(self, other, to_copy, pb)
> 
> Style wise, I think this is harmful : dispatching by type is usually a
> sign that some behaviour or capability hasn't been recognized and put
> into the objects interface.
> 
> In this case, I'm guessing what really matters is can
> copy_multi_helper_parallel actually talk to the other store - not
> whether it is mutable or not (and definately not whether its class is
> ImmutableStore). 
> 
> So I suggest putting some indication of that in the public Store
> interface - so that if I were to write a Store that works from a CD-ROM,
> it would be an ImmutableStore, but not confuse your code.

Actually, I'd submit that the problem is that the class is misnamed;
it's LocalStore, and it's actually an AppendOnlyStore, not an
ImmutableStore.  Actually, it's not necessarily append-only either--
deletion may be legal, too.  Maybe it's an AssignOnceStore?  Anyhow, a
CD-ROM variant wouldn't properly support the current ImmutableStore
protocol, so my code would get confused no matter what.

Now that I think about it, I don't think there are any interesting
properties of ImmutableStore.  We should always invoke
copy_multi_helper_parallel.

However, in the case where we're downloading from one RemoteBranch while
uploading to another, we probably will want to support optimizing on a
class-by-class basis.  Especially when you look at updating revfiles.  I
think trying to introduce abstractions here would lead to extremely
specific 'abstractions'.

> I also note there are no new tests - unit or otherwise - for this.. I
> suspect that remote access is one area where tests are best put in
> early ;).

It's an area that's rather tricky to unit test at all.  Suggestions welcome.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCwHGV0F+nu1YWqI0RApUbAKCGlRJl/cw4FTaJhKuYkOSLRWmGBQCggoxB
OwwdYGsaKZ0+XfK+EEr80Bw=
=7skR
-----END PGP SIGNATURE-----




More information about the bazaar mailing list