Opened 8 years ago

Closed 8 years ago

#1922 enhancement closed fixed (fixed)

make twistd pluggable, nerf mktap

Reported by: tjs Owned by:
Priority: highest Milestone:
Component: core Keywords:
Cc: jerub, spiv, itamarst, exarkun, ralphm, jml Branch:
Author: Launchpad Bug:

Description (last modified by radix)

twistd <subcommand> should work, and use <subcommand> in a way very similar to how mktap uses it, the difference being that twistd will run the service immediately instead of saving it to a file.

Suggested usage:

$ twistd -n web --path .
<startup messages>
$ twistd web --path .
# forks and daemonizes.

Change History (23)

comment:1 Changed 8 years ago by spiv

  • Cc spiv added

comment:2 Changed 8 years ago by itamarst

  • Cc itamarst added

Writing out .tac files is perhaps worth doing, but I don't think it should be a suggested deployment mechanism. See #1490 for my proposal.

comment:3 Changed 8 years ago by exarkun

  • Cc exarkun added

minus one trillion to writing out tac files.

comment:4 Changed 8 years ago by radix

  • Description modified (diff)
  • Owner changed from tjs to radix
  • Status changed from new to assigned

Indeed minus one trillion to writing out tac files. I'm changing the description to leave that bit out. I'm also removing the stuff about not writing a tistd.log or .pid. I've started working on a branch (twistd-plugins-1922) to implement the remaining features.

comment:5 Changed 8 years ago by radix

  • Keywords review added
  • Owner radix deleted
  • Priority changed from normal to highest
  • Status changed from assigned to new

Ok I think maybe this is ready for review in twistd-plugins-1922.

comment:6 Changed 8 years ago by radix

  • Description modified (diff)

comment:7 Changed 8 years ago by ralphm

  • Cc ralphm added
  • Owner set to ralphm
  • Status changed from new to assigned

comment:8 Changed 8 years ago by exarkun

  • Keywords review removed
  • Owner changed from ralphm to radix
  • Status changed from assigned to new

This needs to add some new unit tests for the new behavior.

I suggest at least these two tests:

  • One which grabs subCommands directly and asserts that subcommands for a set of test plugins are provided. The test plugins can live in a test directory, and you can parameterize the package to look plugins up in on the ServerOptions class.
  • One which calls twisted.scripts.twistd.getApplication directly with various configurations and asserts something about the returned Application

comment:9 Changed 8 years ago by exarkun

Oops. I completely missed the added test_twistd.py.

Revised comments:

  • use file instead of open
  • don't delete the tap file in tearDown
  • I'd still like to see a test that actually results in getPlugins being called, but the tests here are pretty decent, so I guess that's optional.
  • add test-case-name to scripts/twistd.py

Merge when done

comment:10 Changed 8 years ago by radix

  • Resolution set to fixed
  • Status changed from new to closed

(In [17926]) Merge twistd-plugins-1922

Author: radix
Reviewer: exarkun
Fixes #1922

The first step towards a mktapless world.

"Oh, ye so fiercely tended,

Ye little seeds of hate!

I bent above your growing

Early and noon and late,

Yet are ye drooped and pitiful,--

I cannot rear ye straight!"

twistd now can accept subcommands equivalent to mktap's;
it will look up the IServiceMaker plugin for the given subcommand
and use it in precisely the same way that mktap does, except that it
immediately applies the service to an Application and starts it.

That is to say,

"twistd web --path ."

comment:11 Changed 8 years ago by radix

(In [17959]) Revert r17926 - doesn't work on Windows (Fuck Yes I Said It)
Refs #1922

comment:12 Changed 8 years ago by radix

(In [17962]) Reviving dead branch twistd-plugins-1922
Refs #1922

comment:13 Changed 8 years ago by radix

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:14 Changed 8 years ago by radix

(In [17965]) *really* revive twistd-plugins-1922
Refs #1922

comment:15 Changed 8 years ago by jml

  • Cc jml added

comment:16 Changed 8 years ago by radix

  • Keywords review added

I think this is ready for review again. It probably works on Windows now, as I've now got unit tests that test the code there, but I haven't tested it interactively.

comment:17 Changed 8 years ago by radix

ahem, and the branch is twistd-plugins-1922-2.

comment:18 Changed 8 years ago by exarkun

  • Keywords review removed
  • Status changed from reopened to new

ApplicationRunner.createOrGetApplication should comment its usage of loadedPlugins, describing how iterating over subCommands initializes and populates it.

UnixApplicationRunner and its methods should have docstrings.

Any reason not to make setupEnvironment, startApplication, and startLogging (private) methods the Runner classes?

Nothing tests that preApplication and postApplication are ever called or are called in the right order with respect to makeService.

Itamar thinks there should be a subprocess test for twistd (but I think it'd be terrible).

Seems to basically work on Windows.

comment:19 Changed 8 years ago by radix

  • Status changed from new to assigned

The private functions in _twistd_unix: I'm not changing their API because certain third parties have decided to rely on them. When a more concerted effort is made to refactor twistd to have a decent internal design, then it will be more worthwhile to break those APIs.

Thanks for the review, I'm working on the other issues.

comment:20 Changed 8 years ago by radix

  • Keywords review added
  • Owner changed from radix to exarkun
  • Status changed from assigned to new

Ok I've done everything you required in your review.

comment:21 Changed 8 years ago by radix

  • Keywords review removed
  • Owner changed from exarkun to radix
  • Status changed from new to assigned

exarkun gives +1:

[17:01] exarkun radix: you don't know how to spell privileges
...
[17:03] exarkun fix that and then I guess merge

comment:22 Changed 8 years ago by radix

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [17992]) Merge twistd-plugins-1922-3

Author: radix
Reviewer: exarkun
Fixes #1922

The first step towards a mktapless world: take three.

"Oh, ye so fiercely tended,

Ye little seeds of hate!

I bent above your growing

Early and noon and late,

Yet are ye drooped and pitiful,--

I cannot rear ye straight!"

twistd now can accept subcommands equivalent to mktap's;
it will look up the IServiceMaker plugin for the given subcommand
and use it in precisely the same way that mktap does, except that it
immediately applies the service to an Application and starts it.

That is to say,

"twistd web --path ."

comment:23 Changed 3 years ago by <automation>

  • Owner radix deleted
Note: See TracTickets for help on using tickets.