On 26 Jul, 09:47 pm, paulswartz@gmail.com wrote:
On 7/26/07, glyph@divmod.com <glyph@divmod.com> wrote:
On 09:14 pm, paulswartz@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?