Allow twistd to "run" packages, e.g. 'twistd run mypackage --port 8080'

[itamarst]

This is an alternative to making twistd into a library.

Use cases:
1. Starting application from configuration framework, persistent database
2. "I want to pass my own command-line options to twistd"

== Option A ==
twistd gets a new command line option --loader=<name>. Plugins can be registered
for it; they have a usage.Options subclass, a callable which accepts instance of
that, and a name. The callable should return an Application instance. All
options after the --loader=<name> are passed to the Options instance of that
plugin, which may require small changes to Options code.

Examples:
twistd --loader=coil foo.coil
twistd --loader=mantissa quotient.db
twistd --loader=zope foo.zcml
twistd --loader=myapp --port=9080 --baz
twistd --loader=mktap web --port=8080 --path=/tmp

== Option B ==
As above, but instead of using plugins, just point at a module that conforms to
the current mktap tap factory module interface.

Benefits:
1. Less boilerplate for developers, as they don't need to make plugin.
2. Easier for new users to figure out what code is being called.
3. Uses Python's namespaces.

Problems:
1. No introspection on available loaders.

Examples:
twistd --loader=coil foo.coil
twistd --loader=mantissa quotient.db
twistd --loader=zope.zcml foo.zcml
twistd --loader=myapp --port=9080 --baz
twistd --loader=twisted.web.tap --port=8080 --path=/tmp

[glyph]

In this example, how would the 'mantissa' (axiom, actually) loader change the
default location of pid and log files?  because it needs to.

The other problem with not making twistd into a library is that people don't
want to type long command lines; your command-name is part of your UI. 
"axiomatic mantissa; axiomatic start -n" is bad enough, I don't think that
"axiomatic mantissa; twistd -n --loader=mantissa mantissa.axiom" is really a
step up.

[itamarst]

I don't think long command line options are a significant problem. However, how
about:

== Option C ==
Can be implemented as A or B, but the Options is a subclass of the standard
twistd Options class (or something, that won't actually work in practice). First
argument is the loader plugin name/module depending on whether this is based on
A or B.

Example:
axiomatic mantissa; twistd axiom -n # can do same magic that "axiomatic start"
does to find mantissa.axiom
vs.
axiomatic mantissa; axiomatic start -n # this is actually longer to type, HOORJ

[itamarst]

James suggets the simplification for versions A and B: instead of having a
usage.Options instance, the callable accepts a list of strings, the remaining
command-line options, allowing people to use their own command-line parser if
they wish.

[itamarst]

Changed title to reflect the current goal.

[itamarst]

Actually, let me summarize latest goal again:

twistd <somepackage> [args]

Where <somepackage> is an importable package, e.g. "twisted.web" or "quotient". The code will be something like:

import somepackage
svc = somepackage.makeService(nontwistdargs)
runService(svc)

E.g.

$ twistd twisted.web2 --path=/tmp --port=8080
$ twistd someapp --config=my.config

[itamarst]

To clarify, I am proposing something like using module name (e.g. "twisted.web", "twisted.protocols.ftp") as alternative way to finding the tap module interface used by mktap and twistd. That module would be expected to support thr tap module interface.

[exarkun]

trac doesn't version the description field. editing it destroys information :(

[exarkun]

Difficulties with a fully-qualified Python name approach:

• More typing
• No way to list all possible values
• Confusing to users - why does twisted.web2 work, but twisted.internet doesn't?
• Confusing to implementors - why do some parts of Twisted use plugins and others use bare names?
• Using names like twisted.web2 means putting code into twisted/web2/init.py - at least to import makeService from somewhere else

Advantages:

• Simpler application level API than plugins
• Potentially less work to do at runtime (although I'm not certain about this)

[jknight]

Replying to itamarst: He meant to put this:

#1922 partially makes this irrelevant, but still... why *do* we require plugins for mktap? Why not a Python module path? Especially given that we no longer persist the application, which means non-stable module references is less of a problem.

in the comment field, not the description field (which was blank before).

[itamarst]

More typing seems not too important, especially most packages won't work via subpackages I suspect so it'll be same length (and same value probably); twisted is an exception. No way to list all values is a valid argument. I'm not sure having code in init.py is a big deal.

As far as confusing implementors, this seems way much much simpler. Installing a package for my proposal would be easier since you don't have to figure out how to install twisted/plugins/ (do we have documented distutils extensions for that?). The confusion would just come from Twisted, and if we go down this route maybe current plugins should be converted to use this method and leave plugin support only for backwards compat.

Not being able to list the available services does seem to be the main problem. BTW, does the new twistd have an option for listing them?

[radix]

It indeed has an option for listing them. "--help".

[exarkun]

Summarizing consensus achieved in last night's meeting:

• twistd pluginname will be kept and gently encouraged
• twistd run will be a Twisted-provided plugin which takes one additional argument, the FQPN of an object providing IServiceMaker
• The introductory documentation will explain twistd run first. A separate document will explain the steps for converting an application written in this style to an application written in the plugin style.
• The run plugin will be written in a way so as to be useful as an example for writing twistd plugins.

radix is working in this area, it seems, so I'm giving this ticket to him for the time being.

[therve]

For fun and profit, I did a quick hack that seems to accomplish that.

[radix]

hey therve, write a unit test or fifteen for the code you wrote

[therve]

(In [23265]) Branching to 'run-plugin-1490'

[therve]

I added the tests. You take care of the documentation ? :)

[radix]

Why do we need to support both 'options' and 'Options'?

I've improved the branch a slight bit. I'll try to get to howto hacking soon, but 8.0.2 takes priority.

[therve]

I supported Options so that 'twistd run twisted.web.tap' worked. Maybe this is not mandatory, though.

[therve]

(In [23291]) Add a simpler interface, add some basic documentation.

Refs #1490

[itamarst]

Glad you're working on this. I do still prefer the original idea where it would look for the service factory on the module level, so you could do:

twistd run myproject.service --port 8080

rather than

twistd run myproject.service.serviceMaker --port 8080

though I suppose myproject/init.py could import serviceMaker and give it a shorter name... and actually I guess a module could provide the interface. So never mind.

[therve]

I have added some documentation to explain how to do what itamar suggested. I think this is ready to review, even though that should not be in 8.1. Every feedback is appreciated, especially on documentation.

[exarkun]

• doc/core/howto/tap.xhtml
  • the first paragraph about twistd run would probably benefit from a bit more markup - twistd run, options attribute, makeService method, twisted/plugins directory, fill service.py.
  • In the 3rd sentence in that paragraph, it just need an options should be it just needs an options.
  • I might reverse the order of the examples, first introducing the most simple way to implement the interface, then mentioning that if it is necessary to keep some state or if it is desired to have multiple services which are very similar but not exactly the same (ie, something multiple instances of one class providing the interface could provide) then bringing in the slightly more complicated example involving a class. This keeps the number of ideas a newbie encounters to an absolute minimum while providing them with something they can actually use. Then when they learn the more complicated way, they already know something, so they just need to adjust their understanding a bit to get the more complex idea. Likewise, this idea would suggest that twistd run should be introduced before the more complicated plugin-based system (which currently appears before this section in the tap howto). This also provides what might be another advantage - the simpler code can be expanded slightly to turn it into a plugin-style service, instead of starting over from scratch. Of course, deciding what approach will best convey information to someone trying to learn is very difficult, and I could be completely wrong about this change being an improvement on what's there already.
  • Maybe it's worth mentioning that services written this way can't be discovered automatically, unlike those implemented via plugins? I don't know if that will surprise anyone. Alternatively, if twistd run is introduced first, then this can be skipped and later when the plugin-style is introduced, enumerability can be explained as a feature it has which goes beyond twistd run.
• twisted/application/app.py
  • ISimpleServiceMaker doesn't seem like the best name. Unfortunately it seems like the existing IServiceMaker forces this a bit. I guess I'm going to keep thinking about this a bit, and maybe I'll have a suggestion to make later on.
  • I would leave evolved out of the docstring of IServiceMaker and just describe the functional difference (ie, that it is discoverable as a plugin by twistd, which you have basically already explained there).
• twisted/test/test_twistd.py
  • twisted_run import should be at the top of the file
  • name TestOptions, TestServiceMaker, testServiceMaker, TestTap, and testTap with the Stub or Dummy or Fake prefix instead. Also, TAP is an acronym, so should be capitalized in those names.
  • maybe testService and testTap could be created in the first place by setUp and deleted entirely in tearDown, instead of re-using and cleaning the same instances?
  • test_unknowOptions is missing a letter
  • there should be a test that run is a subcommand of twistd (ie, that the run plugin is found by getPlugins). it might also be nice to use verifyObject on the RunOptions instance here.

[therve]

Thanks for the comments. I tried to improve the documentation per your suggestions, I hope it's going in the right direction. Every else is good, except the ISimpleServiceMaker name :)

[therve]

(In [23740]) Branching to 'run-plugin-1490-2'

[exarkun]

I attached some doc changes that might be good. It'd be nice to have some other eyes on this too.

The only other code change I'd suggest now is that we might not want to add a new file for RunPlugin. Really, all of core's plugins could be in one file (probably one that imported a few names from elsewhere and didn't actually define anything itself). We seem to have grown quite a few extra files for core in twisted/plugins. Maybe consolidating them is part of a different ticket, though.

[radix]

There has been lots of conversation recently among exarkun, glyph and I about simplified deployment APIs. Given that twistd run is all about simplifying deployment, I think it's worth dumping here.

One point is that #3270 has a branch which includes a new 'react' function which helps in the deployment of programs which have a main function that takes a reactor and returns a Deferred. The program will exit when the Deferred fires.

This is an important use case that we should definitely take into consideration for simplified deployment, and I think at the least 'twistd run' should support something like it. I don't think 'react' should go in as-is because it's Yet Another deployment API which has severe limitations (for example, you can take advantage of common reactor choosing and logging code) and if we're going to be putting something into example code we should make sure it's actually something we want people to use.

Glyph also has some code that he called 'mainpoints' which allows you to decorate your main function and allow it to be used both on the command line and via a twistd plugin. I think this is closer to what I want, but I think it needs more consideration and tweaking.

Given all this interesting discussion of deployment APIs, I am slightly wary of having *this* ticket's branch merged, because I really don't want to introduce so many incompatible, limited deployment APIs all at once and make the "twisted deployment" scene even more complicated and horrible than it already is. I think we should have some discussions for weighing pros/cons of all of these different deployment APIs and at least understand what they are before blessing them with presence in trunk.

[therve]

Back to back.

[exarkun]

I didn't look at the doc changes, since ISTR that I wrote some of them, so someone else should probably take a look at those.

The command line would probably benefit from some different handling of the case where an FQPN which doesn't exist or which doesn't name an ISimpleServiceMaker provider is given, showing something other than a Python traceback.

[therve]

(In [24610]) Check for the specific interface to give a proper error message. Add moduleProvides for a bunch of tap module.

Refs #1490

[therve]

I explicitly added check for the interface in the plugin. I've added moduleProvides on some tap module, I'll add it to all if it's an accepted solution. Back to review.

[exarkun]

Looks pretty good.

I dunno about the moduleProvides in the existing tap modules. There's already a way to run all of them. Does this really add much of value? I'm not really strongly against it. If we decide to do it, there should be tests, though.

Some of the tests could use TestCase.patch to be a little nicer, if you want. Definitely not a big deal.

I guess I did some bad things to the docs. There shouldn't be whitespace inside <code class="API"></code> tags, it messes up the generated links. tap.xhtml has this in a couple places now.

The remaining issues seem very minor now. Maybe we can get some other people involved in the discussion about having existing tap plugins also provide ISimpleServiceMaker to resolve that question.

[therve]

See comment:9 for the proposal to use support it for tap plugins.

[therve]

So I removed the tap support, use patch when I saw a use, and fix API link I think. Put it into review so that glyph can take a look at docs.

[glyph]

I think this is too complex. The original explanation of the requirements is somewhat muddled, because it uses large, established packages like "twisted.web" and "quotient" as examples. There's really no reason for these packages to use 'twistd run'; after all, they already provide plugins. From a user-experience perspective, plugins are better in every way; they're discoverable, they are self-documenting.

The real goal of a plugin like 'twistd run' is to serve as a sort of analogue to running 'python' on a simple little script to try something out. Right now, if you want to run a quick Twisted example, you have a few options.

• You can write a plugin, but that is a lot of work: you have to add an entry to sys.path, write a bunch of classes to satisfy the plugin interface, and so on.
• You can write a .tac file, but then you have no way to handle command-line arguments - or any other form of input, for that matter - so you have to keep modifying your source code if you (for example) want to run your example as a client against different servers.
• You can call reactor.run() at the end of your script, but then you lose logging, daemonization, the ability to quickly reconfigure which reactor is being used (and so on, for all the features of twistd).

This is especially problematic for documentation examples, which want to focus on what the application code is doing, not on how to set up the reactor, or logging, or handle command-line arguments. Our documentation examples should be written to run with twistd run.

twistd run should give you the best of all of these worlds. It should be as little work as just calling listenTCP() / reactor.run() at the end of a Python script; it should let you handle your command-line arguments (ideally however you want, involving twisted.python.usage is often more baggage than it's worth if you have a command-line that just takes one argument), and it should provide you with the many splendors of twistd.

A good litmus test for this is the documentation. The documentation should begin with an example where the user has to do no more than write a single python file. They should not need to modify their sys.path, and ideally they should not need to import anything beyond the core Twisted features required for their application code. This should work on either module or file names; I might have a module in my pre-existing project that I want to quickly run, or I might have a new file with no associated dependencies.

The documentation does need two parts though: writing a module that can be run by 'twistd run', and then evolving that module into a plugin, where it can be discovered and documented and so on.

I believe an ideal interface for this would be for the module in question to have a 'run' method defined in your module which takes a list of arguments and returns a service; even nicer would be to allow for returning a Deferred too, to make it easy to write examples which run, demonstrate something, and then terminate gracefully without needing to call reactor.stop() in examples (and giving the misleading impression that it's normal to terminate the reactor after your application has finished doing something).

[exarkun]

A run method that returns a service? For serious?

[glyph]

Replying to exarkun:

A run method that returns a service? For serious?

Okay, yeah. That was pretty dumb. But the rest of what I said stands :-).

[itamar]

I want to get this done, but don't want to do all the work and then get design thrown out. I do like the idea of absolute minimum knowledge required, which means using services probably also shouldn't be required: another thing to learn. Likewise, a special function name like 'run' is another special thing, and makes it harder for new users to find the entry point.

How does this sound?

"twistd run package.module.func --arg1 x" will call package.module.func. Two arguments are passed in to this function, reactor and the list of extra commandline arguments. Thrown exceptions will be printed to stderr after which twistd will exit. Deferred can optionally be returned; when it fires the reactor will shut down.

I don't know if the branch currently does so, but I would suggest replacing .tac documentation with this, assuming #638 (and perhaps the proposed extra ticket for logging options) has been merged.

[glyph]

Replying to itamar:

I want to get this done, but don't want to do all the work and then get design thrown out. I do like the idea of absolute minimum knowledge required, which means using services probably also shouldn't be required: another thing to learn. Likewise, a special function name like 'run' is another special thing, and makes it harder for new users to find the entry point.

Woohoo design review.

How does this sound? "twistd run package.module.func --arg1 x" will call package.module.func. Two arguments are passed in to this function, reactor and the list of extra commandline arguments. Thrown exceptions will be printed to stderr after which twistd will exit. Deferred can optionally be returned; when it fires the reactor will shut down.

I'm almost on board with this. I think that we should also have some facility to run a file, rather than a module. The use-case is that of small examples; thinking of everything in terms of fully-qualfiied module names will cause problems for people who don't yet understand how Python's module system works (i.e. almost everyone learning Twisted). The biggest conundrum this raises for me is that if one is to use a module, then having a function within that module makes sense; but if we want twistd run filename.py to work, that may imply a script without a main function. Should these just be different plugins?

The second, much less important point: why the reactor and not the other global state (for example, everybody's favorite: logging) that the twistd command line sets up?

[itamar]

Re logging, the current proposal is to use the twistd command line for that, so I'm not sure if there's any point having the user program override it (the user program can always *add* another observer). I'm open to counterarguments though.

I want reactor in the argument list since I'm tired of answering questions about the difference between TCPService and reactor.listenTCP :) Plus we want explicit reactors everywhere.

As far as running files as opposed to modules, that's a good use case. Especially since would make running quick scripts with twistd much easier, which is a plus. I'm tempted to suggest something horrible involving overriding sys.argv, setting __name__ to 'main' and turning reactor.run() into a noop or something, thereby letting you run most non-twistd twisted programs... but that would be horrible, so I won't.

How about main(reactor, argv) as an entry point? Also I realized the returned Deferred could in theory let us implement the oft requested ability to determine the exit code.

[radix]

I'm not totally against itamar's most recent proposal, but I think I would prefer that 'run' just use exactly the same API as the twistd plugin system (so 'twistd run mypackage' would just call 'mypackage.twistd_run()' and treat the result as it would treat the twistd plugin). If everyone else thinks this is a bad idea, though, then fine.

However, one thing that I really think is that we should NOT require the user to type in the FQPN of the function to call; that's just way too programmery and every time I use some third-party thing that makes me type the fully qualified names of, e.g., java classes, I puke a little. I'd much rather have 'run' look up a well-defined name on the object specified on the command line.

I have some time today and probably in the next couple of days to work on this.

[radix]

(In [32237]) Branching to 'run-plugin-1490-3'

[itamar]

If it's similar interface to plugins then therve's branch basically implements that, so not much work left to do. The key is agreeing on the API. How about a compromise?

"twistd run mypackage.module ..." looks at mypackage.module for object named "main" and calls it. We provide and document a utility function that can use a ISimpleServiceMaker (the plugin interface, basically).

This means you can do files (glyph's use case), simple functions, but also plugin-like things with services. So progressing from quick hack to full plugin can be done in stages, and this will be reflected in the documentation as well.

[itamar]

Summary from IRC:

Use cases:

1. Minimal new knowledge to learn in order to use twistd.
2. Run files in addition to modules.
3. Use IServiceMaker without having to create plugin.

We decided to ignore second use case for now, and to approximage the following API:

def main(reactor, argv):
    reactor.listenTCP(int(argv[0]), MyServer())

or

def main(reactor, argv):
    return TCPServer(int(argv[0]), MyServer())

or

main = HeliocideHelper(ServiceMaker("myproject.tap", "whatever" ...))

[glyph]

Replying to itamar:

Summary from IRC: Use cases: 1. Minimal new knowledge to learn in order to use twistd. 2. Run files in addition to modules. 3. Use IServiceMaker without having to create plugin. We decided to ignore second use case for now, and to approximage the following API:

I think that this API works fine if we want to support the second use-case as well. But we can defer that to a separate ticket.

Also, just to make sure I understand properly, the API is that you put some code into a module, 'foo.py', and then you 'twistd run foo', which looks up a function in 'foo' with a predetermined name (we think 'foo' is pretty good). Said function will be called with the reactor and the arguments passed to 'run'. If the module looks like this:

def main(reactor, argv):
    reactor.listenTCP(int(argv[0]), MyServer())

i.e. the function returns None, then it will simply be called by privilegedStartService of a special service and the reactor will run on its merry way.

If the function looks like this:

def main(reactor, argv):
    return TCPServer(int(argv[0]), MyServer())

i.e. the function returns an IService, then it will be called by privilegedStartService of a special service, which will notice that it is an IService, and insert it into the service hierarchy so that privilegedStartService, startService, et. al. will be called on it as expected.

If it instead looks like this:

main = RunServiceMakerHelper(ServiceMaker("myproject.tap", "whatever" ...))

(HeliocideHelper was not a useful name outside the context of the IRC discussion.)

RunServiceMakerHelper returns a callable object which will take the reactor arglist and do what ServiceMaker normally does in a plugin; i.e. build an IService from command-line arguments using a usage.Options subclass. The tapname will be ignored, of course. Note that --help (and maybe one or two other features of Options?) may need to be special cased by the run subcommand to make sure that it can make it into the arglist.

[radix]

We can maintain this API, but your implementations suggestions won't work, glyph.

We can't run 'main' from privilegedStartService, because that introduces two problems:

- main won't be able to print any text to stdout - main won't be able to exit gracefully.

This is all because privilegedStartService happens after daemonization. These features are pretty necessary if we want runnable modules to print help about their arguments and exit when not enough or bad arguments are provided.

So right now the implementation simply calls main() directly in RunPlugin.makeService(), which happens before daemonization and gives the main a chance to do those things. This also makes the main-returning-Service case simpler to implement, because RunPlugin.makeService can just return the service that main returned.

Are there any foreseeable problems with this strategy?

[itamar]

Note that #638 has been merged, and #5212 opened for extended twistd custom logger configuration. These two tickets cover the logger customization functionality that .tac files provide.

[exarkun]

fwiw, I resubmitted #3270 for review.

[itamar]

1. As exarkun notes (implicitly), we perhaps should do something compatible with #3270, allowing an easy transition from simple scripts to twistd. The only inelegant thing about that is that if you no exit condition, you have to return a Deferred you drop on the floor. Perhaps #3270 should be extended before the next release when it crystallizes so that returning None means "don't exit?".
2. Service running could be done using Glyph's suggestion of twisted.application.service.runWithReactor (AFAIK no ticket?). If we go that route, we can just descope services from this ticket.
3. Running before daemonization is a problem. For example, thread pools started before the fork won't survive daemonization. So given wanting to be compatible with task.reactor, the code needs to run after the fork (i.e. startService).

[itamar]

Urgh.

If we want to be able to respond to bad command-line arguments *before* forking, when you still have stderr/stdout, command-line parsing code has to be done as root. But then you hit the problems with e.g. thread pools which as I said break if you naively create them before daemonizaton.

[itamar]

s/as root/before forking/.

[exarkun]

If we want to be able to respond to bad command-line arguments *before* forking, when you still have stderr/stdout,

Another idea is that you don't have to throw away stderr/stdout until you've finished setup and won't have any more startup errors to report. There's another ticket somewhere that talks about this feature (I don't think I'll try to find it right now (hell, it may even be this ticket, there's enough history on it that I can't easily grasp it all)). There are a number of possible approaches to this. One is to just preserve the files across the various process operations until you don't need them any more. Another is to leave the parent running with a communication channel to all of its children until startup is done, and make the parent report any problems its children communicate back to it, and only exit after everything is working fine.

[therve]

The ticket which would hep managing the after forking case is #823.