[Twisted-Python] potential connectSSH workflow

[glyph at divmod.com]

On 26 Jul, 09:47 pm, paulswartz at gmail.com wrote:
>On 7/26/07, glyph at divmod.com <glyph at divmod.com> wrote:
>>On 09:14 pm, paulswartz at gmail.com wrote:

Let me quote this point from the end first, because it is the central 
point of agreement, and basically the specification for this feature:
>This would make the default .connect(hostname) work equivalently to
>the default 'ssh hostname' but still give options for the other
>overrides.

I don't think, however, that the distinction being proposed (function 
arguments vs. defaults) is quite the right way to go about it, though.
>> >That makes sense: a ConchConnection object (snip)

>>"Connection" seems like a weird name for that to me, as it applies it 
>>is
>>attached to a particular ... well, connection.  Connector, maybe?

>I actually meant Connector :) There's also Creator (in comparison with
>t.i.protocol.ClientCreator).

Let's call it ConchEndpoint for the moment.  Hopefully why I say that 
will become clear in a moment.
>> >I still think that passing
>> >authentication data in connect() is the right idea, but it can 
>>default
>> >to using the keys that it finds in ~/.ssh or in a key agent.

>>I agree emphatically, but both the default and non-default cases are
>>equally important.

>class IConch<C word>(Interface):
>    def connect(host, port=22, user=None, knownHosts=None,
>authentications=None):

I think the interface here is probably superfluous.  The interface 
definition, when there is one, should really just be an endpoint.  These 
may make sense as constructor arguments or arguments to a method that 
builds an endpoint though.  (More below...)
>To keep the current use case of verifying a host key with a function,
>knownHosts could be a filename string, in which case it would be read
>for host keys, or a function in which case it is called with the key
>and fingerprint.  authentications would be a list functioning the same
>was as in my initial outline.

OK, let's have a bit more depth.  This interface isn't quite right.

Starting at the bottom, "knownHosts" is the wrong name for this object. 
It should be "hostVerifier" and it should be an object with a method to 
verify the host.  It should definitely _not_ accept a filename string; 
in the filename case, it should look something like this:

    OpenSSHFormatVerifier(FilePath("my-ssh-hosts-file")
                          ui=GuiPasswordDialog())

In general, I think passing filenames around should be deprecated within 
Twisted; FilePath provides lots more flexibility, especially with a real 
interface to unify it with ZipPath.  (A marginal benefit of this is that 
your tests can avoid a whole bunch of file I/O because you can have a 
mock memory-backed FilePath.  This benefit becomes less marginal as more 
and more tests use it and the whole suite speeds up, though.)

As far as object-vs-function, passing functions around is always 
tempting, but it tends to mean "This is an object with only one obvious 
method that I've thought of so far, which I'll call '__call__'".  There 
are lots of places I've used this idiom and found that it falls down and 
gets ugly when it ends up that you actually want _two_ methods instead.

To retain compatibility with the existing use-a-function-to-verify path, 
it would of course be trivial to have a SimpleFunctionVerifier.

This might seem like a lot of typing, so to help with abbreviation, a 
higher-level object could roll them together:

    altdotssh = DotSSH(FilePath("my-dot-ssh-dir"))

And this leads in to my idea for subtly changing the design.  Instead of 
modifying stuff with arguments to the function:

    defaultConnector.connectSSH(
        factory, hostname, hostVerifier=altdotssh.hostVerifier)

Building a different object takes roughly the same amount of work and 
does roughly the same thing, but yields a result which can be passed 
around and used as a connector:

    endpt = defaultConfig.endpointSSH(
        hostname, hostVerifier=altdotssh.hostVerifier)
    endpt.connect(factory)

To push this flexibility even further, you could make the configuration 
object cloneable.

    cfg2 = defaultConfig.copy(hostVerifier=altdotssh.hostVerifier)
    endpt = cfg2.endpointSSH(hostname)
    endpt.connect(factory)

although whether this last additional level of flexibility is worth it 
or not probably depends on all of the things that a "configuration" 
object can contain.

Thoughts?