dulwich-users team mailing list archive

[Jelmer Vernooij <jelmer@xxxxxxxxx>]

On Tue, 2010-12-21 at 07:05 -0600, Augie Fackler wrote:
> On Dec 21, 2010, at 4:22 AM, Jelmer Vernooij wrote:
> > 
> > On Tue, 2010-12-21 at 12:46 +0800, Tay Ray Chuan wrote:
> >> Allow the vendor to be specified on the client class. That way, when
> >> users wish to use a separate vendor, they can just subclass
> >> SSHGitClient, instead of replacing (globally) client.get_ssh_vendor.
> >> ---
> >> dulwich/client.py                   |    7 +++----
> >> dulwich/tests/compat/test_client.py |   25 +++++++++++++------------
> >> 2 files changed, 16 insertions(+), 16 deletions(-)
> >> 
> >> diff --git a/dulwich/client.py b/dulwich/client.py
> >> index 2b69852..e89879f 100644
> >> --- a/dulwich/client.py
> >> +++ b/dulwich/client.py
> >> @@ -338,12 +338,11 @@ class SSHVendor(object):
> >>                                 stdout=subprocess.PIPE)
> >>         return SubprocessWrapper(proc)
> >> 
> >> -# Can be overridden by users
> >> -get_ssh_vendor = SSHVendor
> >> -
> >> 
> >> class SSHGitClient(GitClient):
> >> 
> >> +    ssh_vendor_class = SSHVendor
> >> +
> > My main concern is about keeping compatibility with the Bazaar SSHVendor
> > interface - both so bzr-git can easily hook it in, but also to keep the
> > option open of stealing that code easily in the future. This patch
> > doesn't change the SSHVendor class, so that's not really an issue here. 
> > 
> > I'm happy to merge this but would prefer to keep calling this variable
> > get_vendor as ideally it selects a vendor (openssh, sshcorp, plink, lsh,
> > paramiko, etc) and returns an instance of that rather than being
> > hardcoded to a single class. 

> I won't bikeshed what to call it, but IMO we need to not remove
> get_ssh_vendor in the same version of dulwich that we move to some
> other interface. Did you see my feedback on this patch? Do you have any
> thoughts on that? (I wanted to make it possible for clients to not
> *have* to implement bzr's ssh vendor scheme if that's undesirable.)
No, I hadn't seen that at the time I replied. I think that's a good
suggestion.

I presume you'd like to override it with whatever mechanism you have in
Mercurial for creating a SSH connection?

Cheers,

Jelmer

Attachment: signature.asc
Description: This is a digitally signed message part