CLI: allow dispatch functions to return Deferred

In addition, CLI functions are allowed to use sys.exit() instead of
always needing to return the exit code as an integer.

runner.py now knows about the blocking httplib calls in scripts/cli and
scripts/magic_folder, and uses deferToThread() to invoke them. Those
functions cannot return a Deferred: when rewrite them to use twisted.web
or treq, we'll remove this deferToThread call.

Option parsing was split out to a separate function for testing. We now
use twisted.internet.task.react() to start the reactor, which required
changing the way runner.py is tested.

closes ticket:2826

src/allmydata/scripts/runner.py

@@ -3,6 +3,7 @@ import os, sys
 from cStringIO import StringIO
 
 from twisted.python import usage
+from twisted.internet import defer, task, threads
 
 from allmydata.scripts.common import get_default_nodedir
 from allmydata.scripts import debug, create_node, startstop_node, cli, \
@@ -89,23 +90,17 @@ create_dispatch = {}
 for module in (create_node, stats_gatherer):
     create_dispatch.update(module.dispatch)
 
-def runner(argv,
-           run_by_human=True,
-           stdin=None, stdout=None, stderr=None):
-
-    assert sys.version_info < (3,), ur"Tahoe-LAFS does not run under Python 3. Please use Python 2.7.x."
-
-    stdin  = stdin  or sys.stdin
-    stdout = stdout or sys.stdout
-    stderr = stderr or sys.stderr
+def parse_options(argv, config=None):
+    if not config:
+        config = Options()
+    config.parseOptions(argv) # may raise usage.error
+    return config
 
+def parse_or_exit_with_explanation(argv, stdout=sys.stdout):
     config = Options()
-
     try:
-        config.parseOptions(argv)
+        parse_options(argv, config=config)
     except usage.error, e:
-        if not run_by_human:
-            raise
         c = config
         while hasattr(c, 'subOptions'):
             c = c.subOptions
@@ -115,14 +110,15 @@ def runner(argv,
         except Exception:
             msg = repr(e)
         print >>stdout, "%s:  %s\n" % (sys.argv[0], quote_output(msg, quotemarks=False))
-        return 1
+        sys.exit(1)
+    return config
 
+def dispatch(config,
+             stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr):
     command = config.subCommand
     so = config.subOptions
-
     if config['quiet']:
         stdout = StringIO()
-
     so.stdout = stdout
     so.stderr = stderr
     so.stdin = stdin
@@ -136,29 +132,46 @@ def runner(argv,
     elif command in admin.dispatch:
         f = admin.dispatch[command]
     elif command in cli.dispatch:
-        f = cli.dispatch[command]
+        # these are blocking, and must be run in a thread
+        f0 = cli.dispatch[command]
+        f = lambda so: threads.deferToThread(f0, so)
     elif command in magic_folder_cli.dispatch:
-        f = magic_folder_cli.dispatch[command]
+        # same
+        f0 = magic_folder_cli.dispatch[command]
+        f = lambda so: threads.deferToThread(f0, so)
     else:
         raise usage.UsageError()
 
-    rc = f(so)
-    return rc
-
+    d = defer.maybeDeferred(f, so)
+    # the calling convention for CLI dispatch functions is that they either:
+    # 1: succeed and return rc=0
+    # 2: print explanation to stderr and return rc!=0
+    # 3: raise an exception that should just be printed normally
+    # 4: return a Deferred that does 1 or 2 or 3
+    def _raise_sys_exit(rc):
+        sys.exit(rc)
+    d.addCallback(_raise_sys_exit)
+    return d
 
 def run():
-    try:
-        if sys.platform == "win32":
-            from allmydata.windows.fixups import initialize
-            initialize()
+    assert sys.version_info < (3,), ur"Tahoe-LAFS does not run under Python 3. Please use Python 2.7.x."
 
-        rc = runner(sys.argv[1:])
-    except Exception:
-        import traceback
-        traceback.print_exc()
-        rc = 1
-
-    sys.exit(rc)
+    if sys.platform == "win32":
+        from allmydata.windows.fixups import initialize
+        initialize()
+    d = defer.maybeDeferred(parse_or_exit_with_explanation, sys.argv[1:])
+    d.addCallback(dispatch)
+    def _show_exception(f):
+        # when task.react() notices a non-SystemExit exception, it does
+        # log.err() with the failure and then exits with rc=1. We want this
+        # to actually print the exception to stderr, like it would do if we
+        # weren't using react().
+        if f.check(SystemExit):
+            return f # dispatch function handled it
+        f.printTraceback(file=sys.stderr)
+        sys.exit(1)
+    d.addErrback(_show_exception)
+    task.react(lambda _reactor: d) # doesn't return: calls sys.exit(rc)
 
 if __name__ == "__main__":
     run()

src/allmydata/test/cli/test_cli.py

@@ -5,6 +5,7 @@ import urllib, sys
 
 from twisted.trial import unittest
 from twisted.python.monkey import MonkeyPatcher
+from twisted.internet import task
 
 import allmydata
 from allmydata.util import fileutil, hashutil, base32, keyutil
@@ -511,9 +512,9 @@ class CLI(CLITestMixin, unittest.TestCase):
         exc = Exception("canary")
         ns = Namespace()
 
-        ns.runner_called = False
-        def call_runner(args):
-            ns.runner_called = True
+        ns.parse_called = False
+        def call_parse_or_exit(args):
+            ns.parse_called = True
             raise exc
 
         ns.sys_exit_called = False
@@ -521,13 +522,23 @@ class CLI(CLITestMixin, unittest.TestCase):
             ns.sys_exit_called = True
             self.failUnlessEqual(exitcode, 1)
 
-        patcher = MonkeyPatcher((runner, 'runner', call_runner),
+        def fake_react(f):
+            d = f("reactor")
+            # normally this Deferred would be errbacked with SystemExit, but
+            # since we mocked out sys.exit, it will be fired with None. So
+            # it's safe to drop it on the floor.
+            del d
+
+        patcher = MonkeyPatcher((runner, 'parse_or_exit_with_explanation',
+                                 call_parse_or_exit),
                                 (sys, 'argv', ["tahoe"]),
                                 (sys, 'exit', call_sys_exit),
-                                (sys, 'stderr', stderr))
+                                (sys, 'stderr', stderr),
+                                (task, 'react', fake_react),
+                                )
         patcher.runWithPatches(runner.run)
 
-        self.failUnless(ns.runner_called)
+        self.failUnless(ns.parse_called)
         self.failUnless(ns.sys_exit_called)
         self.failUnlessIn(str(exc), stderr.getvalue())
 

src/allmydata/test/common_util.py

@@ -2,7 +2,7 @@ import os, signal, sys, time
 from random import randrange
 from cStringIO import StringIO
 
-from twisted.internet import reactor, defer, threads
+from twisted.internet import reactor, defer
 from twisted.python import failure
 from twisted.trial import unittest
 
@@ -28,25 +28,23 @@ def run_cli(verb, *args, **kwargs):
     argv = nodeargs + [verb] + list(args)
     stdin = kwargs.get("stdin", "")
     stdout, stderr = StringIO(), StringIO()
-    d = threads.deferToThread(runner.runner, argv, run_by_human=False,
-                              stdin=StringIO(stdin),
-                              stdout=stdout, stderr=stderr)
+    d = defer.succeed(argv)
+    d.addCallback(runner.parse_or_exit_with_explanation, stdout=stdout)
+    d.addCallback(runner.dispatch,
+                  stdin=StringIO(stdin),
+                  stdout=stdout, stderr=stderr)
     def _done(rc):
-        return rc, stdout.getvalue(), stderr.getvalue()
-    d.addCallback(_done)
+        return 0, stdout.getvalue(), stderr.getvalue()
+    def _err(f):
+        f.trap(SystemExit)
+        return f.value.code, stdout.getvalue(), stderr.getvalue()
+    d.addCallbacks(_done, _err)
     return d
 
 def parse_cli(*argv):
-    # This parses the CLI options (synchronously), and throws
-    # usage.UsageError if something went wrong.
-
-    # As a temporary side-effect, if the arguments can be parsed correctly,
-    # it also executes the command. This side-effect will be removed when
-    # runner.py is refactored. After the refactoring, this will return the
-    # Options object, and this method can be used for success testing, not
-    # just failure testing.
-    runner.runner(argv, run_by_human=False)
-    assert False, "eek, I can't be used for success testing yet"
+    # This parses the CLI options (synchronously), and returns the Options
+    # argument, or throws usage.UsageError if something went wrong.
+    return runner.parse_options(argv)
 
 class DevNullDictionary(dict):
     def __setitem__(self, key, value):