convert twistd options into structured objects for testing and manipulation

[glyph]

Currently twistd converts some options into objects (app.ApplicationRunner et. al., and some options into a series of function calls (runReactorWithLogging, fixPdb, runWithProfiler, etc).

Before twistd does anything with the arguments that it's given, it should convert all of them into a set of introspectable, structured objects, which can be manipulated.

This would be helpful in two areas: testing and configurability by tac files and plugins. Testing would be easier because these objects could be mocked out, to ensure that their methods are called with the correct values in the correct order. Post-hoc or programmatic configuration would be easier because there could be defined points in the execution of the various options where certain objects could be mutated by user code. This could simplify tickets such as #638.

This might change in the course of implementation, but I believe the tasks that should be represented by objects here includes:

• daemonization
• logging
• IServiceMaker location
• reactor selection
• privilege management (uid, gid, euid, chroot?)
• persistence
• profiling
• process naming

Some or all of these things should also be integrated into the service hierarchy, but I leave it as an exercise to the implementor as to how. Depending on the amount of code that will have to be written to do this it may be wise to split it into multiple tickets.

[exarkun]

Simply structuring the options is insufficient to satisfy any configuration use-case, since it does not provide any way to actually define what configuration is necessary or sufficient.

Testing surely would be simplified, though.

[glyph]

I'm not sure I understand your comment.

I understand that there are certain structures this information could be put into which would not, by themselves, provide any way to determine if they were sufficient to configure twistd. However, a well-designed structure would be initialized in such a way that (barring stupid introspection tricks) any TwistdConfiguration object is a complete and valid configuration for a twistd process. So I think I'm agreeing with you when I say that it should be so designed.

However, this particular ticket is an attempt to separate the definition of all the external hooks for overriding the configuration object from the configuration object itself. Thanks to platform requirements (you must bind ports before shedding privileges, for example) each twistd subsystem will need to have its own timing for calling application code to override it, so tickets like #638 should still exist separately; they can just be facilitated by having an existing, well-defined structure to manipulate.

Regardless, as you say, this is probably worth it just for the testing benefits.

[therve]

I don't know by which end start this ticket.

Can you give me some more explanations of the design you want here? Is it just switching from the usage.Options object to another object? Or does that imply more changes, like putting the free functions of app.py into this object?

Thanks.

[glyph]

Each one of the bullet points in the description (copied here because trac is going to eventually unversion the description and destroy all this information):

• daemonization
• logging
• IServiceMaker location
• reactor selection
• privilege management (uid, gid, euid, chroot?)
• persistence
• profiling
• process naming

should be an object with methods on it. There should be a daemonizer which knows about how to start services and when (or whether) to daemonize, a logging object which represents all the different ways twistd dcan deal with login, a ServiceMakerBuilder which can build an IServiceMaker provider in each of the various ways, a ReactorSelector that deals with the plugin system, and so on, and so on.

Each of these is a separate task, so if you are looking for a way to start on this ticket, it would be filing tickets for stuff that is an unstructured mess (a combination of module state and undocumented free functions, for example) and converting it into a clean API, then linking those tickets from here. It would be best if each of those tickets described a specific issue which is currently problematic due to the lack of structure, so we can make sure that the structure put in place is actually useful.

[therve]

(In [20640]) Completely refactor twistd runner to use some objects to manage. Untested bad documented crappy code (tm).

Refs #2571

[therve]

After messing around a bit, there is some code to look at in twistd-options-2571. There aren't many objects yet (5 I think), but some could probably be splitted.

Also, I throw a lot of things that must be kept for compatibility, but it's easier to look at the code this way, I'll a add layer later.

[therve]

The profile options had been done in #2921. I've started the logging part in #3052.

[glyph]

#727 effectively depends on this now.

[glyph]

Since this has just been brought to my attention again now, I figured I'd comment with some thoughts:

therve, thanks for the stellar work on this.

One comment on the stuff in #3052. I think we might need another ticket to allow users to interact with the configuration somehow. The problem that jumps out at me right away is that _getLogObserver is private, but maybe that would be a bad API for applications anyway.

Going over the code again, I'm surprised to see just how much of this is already almost in the right shape; twisted.internet.app.ApplicationRunner - well, its subclasses, really - encapsulate most of the interesting state and behavior.

This ticket is pretty open-ended, so I'm trying to think of how to narrow its focus a bit and make it something resolvable. I'm starting to think that the real core of this ticket has something to do with eliminating all behavior from ServerOptions, and rather than passing it to ApplicationRunner, having a discrete "parsing" phase, which yields an ApplicationRunner object that does not have a reference to a config object (except, perhaps, for compatibility reasons).

As it stands, almost all the right state is in almost all the right places, but passing the config object around to all the phases of startup means "Maybe there is some structure here, but feel free to rummage around in this bag of strings that I'm giving to you if you need anything." Reading the history now, I see that I never made this clear in my previous comments when therve asked about the design. That's what I mean by "structured" objects: not dicts or lists of strings which some other functions or methods will inspect, but self-contained objects with their own attributes of appropriate types, with methods that don't need to look further than those attributes.

If the parsing were a discrete step, and there were a top-level "here is everything your twistd run will do" object, then supporting any customization you could imagine would simply be a matter of adding a public API for the attributes or methods in question.

Although perhaps I'm oversimplifying, because there's also the issue of defining a common API between _twistd_unix and _twistw.

Thoughts?