dulwich-users team mailing list archive

[Augie Fackler <durin42@xxxxxxxxx>]

On Dec 20, 2010, at 10:46 PM, 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

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.

> -
> 
> 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(...)


>             self.host, ["%s '%s'" % ('git-' + cmd, path)],
>             port=self.port, username=self.username)
>         return Protocol(con.read, con.write), con.can_read
> diff --git a/dulwich/tests/compat/test_client.py b/dulwich/tests/compat/test_client.py
> index b632507..63314c5 100644
> --- a/dulwich/tests/compat/test_client.py
> +++ b/dulwich/tests/compat/test_client.py
> @@ -200,30 +200,31 @@ class DulwichTCPClientTest(CompatTestCase, DulwichClientTestBase):
>         return path
> 
> 
> -class TestSSHVendor(object):
> -    @staticmethod
> -    def connect_ssh(host, command, username=None, port=None):
> -        cmd, path = command[0].replace("'", '').split(' ')
> -        cmd = cmd.split('-', 1)
> -        p = subprocess.Popen(cmd + [path], stdin=subprocess.PIPE,
> -                             stdout=subprocess.PIPE, stderr=subprocess.PIPE)
> -        return client.SubprocessWrapper(p)
> +class TestSSHGitClient(client.SSHGitClient):
> +
> +    class TestSSHVendor(object):
> +        @staticmethod
> +        def connect_ssh(host, command, username=None, port=None):
> +            cmd, path = command[0].replace("'", '').split(' ')
> +            cmd = cmd.split('-', 1)
> +            p = subprocess.Popen(cmd + [path], stdin=subprocess.PIPE,
> +                                 stdout=subprocess.PIPE, stderr=subprocess.PIPE)
> +            return client.SubprocessWrapper(p)
> +
> +    ssh_vendor_class = TestSSHVendor
> 
> 
> class DulwichMockSSHClientTest(CompatTestCase, DulwichClientTestBase):
>     def setUp(self):
>         CompatTestCase.setUp(self)
>         DulwichClientTestBase.setUp(self)
> -        self.real_vendor = client.get_ssh_vendor
> -        client.get_ssh_vendor = TestSSHVendor
> 
>     def tearDown(self):
>         DulwichClientTestBase.tearDown(self)
>         CompatTestCase.tearDown(self)
> -        client.get_ssh_vendor = self.real_vendor
> 
>     def _client(self):
> -        return client.SSHGitClient('localhost')
> +        return TestSSHGitClient('localhost')
> 
>     def _build_path(self, path):
>         return self.gitroot + path