make node initialization synchronous

[warner]

This determines the Tub's location before creating the Tub (by allocating ports and detecting IP addresses synchronously), allowing tub.registerReference to happen in the Node constructor, thus allowing node initialization to happen synchronously. This simplifies the code considerably, and allows most configuration errors to be reported on the tahoe start stderr stream (instead of in a logfile after the process has forked off into a daemon).

Refs ticket:2491, although it only resolves the runtime portions of that ticket, not the tahoe create-node aspects.

It also updates the docs for tub.port and tub.location to discourage automatic port-allocation and IP-address-detection, as discussed in 2491.

[meejah]

As far as this goes, I think the patch looks good. +1 for merging.

I do have reservations about how much work is done in Client's __init__ but at least this patch considerably simplifies it so that work at least doesn't involve a bunch of async stuff as well. So, +1 on this patch landing.

The "amount of work in init" issue is probably best discussed on a (separate?) Tahoe ticket, but what I mean is for example self.init_introducer_client is essentially just a factory for IntroducerClient -- it would make more sense (to me) to have Client's ctor insist on getting an IntroducerClient instance passed in, and Client's ctor merely remembers it (self.introducer_client = ic). Whether the "startup code" is synchronous or async then doesn't really matter -- until you have all the requisite instances ready to go, you simply can't create a Client.

Now, I don't fully understand the startup sequence here so this could easily be out to lunch -- I'm mostly basing this on reading Client.__init__ and (some of) the Client.init_*() methods...

On the "pro" side for changing the above is that you'd end up with a bunch of bare factory methods for the different pieces (e.g. create_introducer_client) and move the "real work" from Client.__init__ to e.g. a create_client factory-method. This gives users of Client a lot more control over how it's set up (e.g. for testing, I can then pass in a Mock or other fake for IntroducerClient) while still maintaining a simple, easy-to-use way to get a client (create_client that could take just a basedir like Client.__init__ does right now). It also makes the prerequisites for Client more explicit (because they're constructor args). On the "con" side is probably: that's a lot of refactoring/work for something that's already working ;)

[warner]

Yeah, I think those are all good observations. There's some inter-dependencies that would need to be resolved (e.g. init_storage needs to use the IntroducerClient, both need to use the Tub), but it might be appropriate to manage that by passing references into constructors rather than grabbing them through e.g. self.introducer_client. I suppose it becomes a question of where we put the knowledge of how everything needs to be wired together.. at the moment it's in Client.__init__, but we could switch to a make_client() function instead.

I originally thought new features (like magic-folders, or a backup agent of some sort) could live as Service children of the main Client, and get access to the gateway-ish features they need (specifically the NodeMaker) by doing self.parent.get_nodemaker(). I thought that these new plugin-like things (written against an internal Tahoe API, but not necessarily shipped with tahoe) ought to be Services anyways, so attaching them with setServiceParent(client) was as clean a way as any to give it a consistent way to access Client functionality.

But in this new way of thinking, I guess it'd be better to create those things with a function that accepts an e.g. NodeMaker as an argument? They could still be Services (which helps manage periodic timers, for one), but they'd never use self.parent to find anything, instead they'd only use the references passed in when they were constructed. That'd be easier for mocking during tests, too.

Let's shift this to a trac cleanup ticket.. I've created https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2782 for it.