[MERGE] Refactor loopback/paramiko/openssh/ssh connection opening

Martin Pool mbp at canonical.com
Mon Aug 28 03:09:32 BST 2006


On 25 Aug 2006, John Arbash Meinel <john at arbash-meinel.com> wrote:
> I like the overall refactoring. I think you might be missing a couple
> steps, but nothing critical at this point.

Yes, it can and should definitely be taken further.  However I think
this is clean & useful as it stands and could come in for 0.11.
>
> v-- In general, all of you SSHVendor children have a 'connect_sftp'
> method which connects to SSH and then wraps it with a SFTPClient.

That's generally true, except that for Paramiko the wrapping is done
inside the Paramiko library.  So we may want to have in the base class a
default connect_sftp() that calls _open_sftp_channel() and then wraps
it.

> I think it would make sense to break out the SSH connection, and then
> you don't have to re-implement it in the future. However, these changes
> can be done at a different time, when you actually go to use ssh
> connections.
> > +    def connect_ssh(self, username, password, host, port, command):
> > +        """Make an SSH connection, and return a pipe-like object."""
> > +        raise NotImplementedError(self.connect_ssh)
> > +        
> 
> ^-- I realize you haven't implemented this yet. but it seems you would
> need more than a 'pipe-like' object. Either you need 2 pipes for
> bidirectional support, or you need a socket-like object. Or maybe even a
> real class.

I think he meant socket-like.

(We may need to be clear about just what methods can be called -- recv
like a socket, or read like a file?)

> > +class LoopbackVendor(SSHVendor):
> > +    """SSH "vendor" that connects over a plain TCP socket, not SSH."""
> > +    
> > +    def connect_sftp(self, username, password, host, port):
> > +        sock = socket.socket()
> > +        try:
> > +            sock.connect((host, port))
> > +        except socket.error, e:
> > +            raise ConnectionError('Unable to connect to SSH host %s:%s: %s'
> > +                                  % (host, port, e))
> > +        return SFTPClient(LoopbackSFTP(sock))
> > +
> 
> ^- loopback is fairly unique, but I think it could be:

In passing I think 'loopback' is an imperfect name, because it doesn't
really loop back to where it came from.  Instead the distinguishing
feature is that it runs across bare unencrypted tcp.


-- 
Martin




More information about the bazaar mailing list