dulwich-users team mailing list archive

[Tay Ray Chuan <rctay89@xxxxxxxxx>]

On Tue, Dec 21, 2010 at 1:16 PM, Augie Fackler <durin42@xxxxxxxxx> wrote:
> On Dec 20, 2010, at 10:46 PM, Tay Ray Chuan wrote:
>> 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
>
> For backwards compatibility you shouldn't remove this yet, and it still needs to be used. That said, if Jelmer accepts this patch (I'm +1, but that goes without saying since I think I sent something similar before) we should mark this as a deprecated mechanism though.

In my haste I forgot all about backward compatibility. We'll see what
Jelmer says though - dulwich does have it's fair share of
backward-incompatible changes now and then, so it might not be worth
the trouble here.

>> class SSHGitClient(GitClient):
>>
>> +    ssh_vendor_class = SSHVendor
>> +
>>     def __init__(self, host, port=None, username=None, *args, **kwargs):
>>         self.host = host
>>         self.port = port
>> @@ -351,7 +350,7 @@ class SSHGitClient(GitClient):
>>         GitClient.__init__(self, *args, **kwargs)
>>
>>     def _connect(self, cmd, path):
>> -        con = get_ssh_vendor().connect_ssh(
>> +        con = self.ssh_vendor_class().connect_ssh(
>
> Rather than this, I think it might make more sense to have a method on SSHGitClient that looks like this:
>
>    def make_connection(host, command, port, uesrname):
>        """Returns a file-like object representing the ssh connection.
>
>        Clients of dulwich may subclass SSHGitClient and override this method to easily
>        customize ssh connection semantics on a per-connection basis.
>        """
>        return get_ssh_vendor().connect_ssh(...)
>
>

Good idea.

-- 
Cheers,
Ray Chuan