Merge lp:~jimbaker/juju-jitsu/subcommand-help into lp:juju-jitsu

[Jim Baker]

Adds support for adding help to the toplevel jitsu command. Because the subcommands are implemented as separate scripts, not importable modules, it tries two things:

1. Build a module to exec the script in, as if it were Python code. This does require that any Python scripts adhere to the convention that they use the convention of being side effect free and use __main__, eg something like this code fragment:

if __name__ == '__main__':
    main()

This is the case with current code. If the code can be loaded, it then looks for a specific top-level function, make_parser(subparser)

2. If step 1 fails, it uses the comment lines as the source for help:

#!/path/to/interpreter
# command - short description
# then any optional usage info as comments,
# blank lines terminating, up to a
# Copyright notice

This results in the following output:

$ jitsu -h
usage: jitsu [-h] [--version] subcommand ...

External tools for working with Juju

optional arguments:
  -h, --help show this help message and exit
  --version show program's version number and exit

subcommands:
  Subcommands for jitsu

  subcommand
    help get subcommand help
    capfile returns a capistrano formatted version of the juju status
    deploy uses local repo like the charm store
    get-service-info
                     returns requested service info
    get-unit-info returns requested unit info
    gource converts juju status yaml output into gource log
    open-port opens a port on a service unit or for all units in a
                     service
    run-as-hook runs a script or command as if it were a hook, on this
                     computer
    setup-environment
                     sets up your juju environment interactively
    topodump dumps the zookeeper topology
    upgrade-charm upgrade-charm
    watch waits on Juju environment for specified conditions
    wrap-juju execs an interactive shell with juju wrapped

with detailed help currently available only on jitsu watch --help; the remaining commands can be modified as it makes sense.

[lp:~jimbaker/juju-jitsu/subcommand-help]

65. By Jim Baker on 2012-07-05

Support symlinked commands and move more help processing out of jitsu.in

66. By Jim Baker on 2012-07-05

Merged trunk

67. By Jim Baker on 2012-07-05

Docstrings

[Jim Baker (jimbaker)]

I pushed a new version which does the following:

1) Resolves symlinks so commands can specify their help in comments, but be pointed at by multiple symlinked versions

2) Only attempts the help parsing with make_parser if the command is a jitsu builtin, not from the users plugin directory.

[Mark Mims (mark-mims)]

review: Approve

=== modified file 'bin/jitsu.in'
--- bin/jitsu.in	2012-06-21 00:18:00 +0000
+++ bin/jitsu.in	2012-07-05 17:34:23 +0000
@@ -1,5 +1,6 @@
-#!/usr/bin/python
-# jitsu - wraps juju command with added sub-commands from juju-jitsu
+#!/usr/bin/env python
+#
+# jitsu - external tools for working with juju
 # Copyright 2012 Canonical Ltd. All Rights Reserved.
 #
 # This program is free software: you can redistribute it and/or modify
@@ -15,51 +16,48 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import argparse
 import os
+import sys
 from os.path import abspath, join, dirname, expanduser, isdir, isfile
-import argparse
 from juju.control import JujuFormatter
 
+
+subcommand_home = abspath(join(dirname(__file__),'..','sub-commands'))
+if not isdir(subcommand_home):
+    subcommand_home="@datadir@/juju-jitsu/sub-commands"
+sys.path.insert(0, subcommand_home)  # Ensure the aiki subdirectory can be imported
+
+from aiki.cli import add_command_help, get_commands
+
+
 parser = argparse.ArgumentParser(
   formatter_class=JujuFormatter,
-  description='Wrapper for juju adding sub-commands from juju-jitsu')
+  description='External tools for working with Juju')
 
 parser.add_argument('--version', action='version', version='%(prog)s @version@')
 
-subcommand_home = abspath(join(dirname(__file__),'..','sub-commands'))
-if not isdir(subcommand_home):
-    subcommand_home="@datadir@/juju-jitsu/sub-commands"
-
+# Create a mapping of commands to their details (such as their source
+# directories); this two-step process enables user plugins to override
+# existing commands
+cmds = dict(get_commands(subcommand_home))
 user_subcommands = expanduser("~/.juju-jitsu/plugins")
-
-cmds = set(os.listdir(subcommand_home))
 if isdir(user_subcommands):
-    cmds.update(os.listdir(user_subcommands))
-
-subparser = parser.add_subparsers(description = 'Sub command for juju-jitsu',
-        dest='sub_command')
-
+    cmds.update(get_commands(user_subcommands))
+subparser = parser.add_subparsers(description = 'Subcommands for jitsu', dest='subcommand')
 help_parser = subparser.add_parser('help',
-        description='Get detailed subcommand help',
-        help='get sub command help')
-help_parser.add_argument('help_sub_command', choices=cmds)
-
-for cmd in cmds:
-    # Filter anything not a bare command word
-    if '.' in cmd:
-        continue
-    if cmd == 'Makefile':
-        continue
-    if cmd == 'aiki':
-        continue
-    cmd_parser = subparser.add_parser(cmd, description=cmd, help=cmd)
+                                   description='Get detailed subcommand help',
+                                   help='get subcommand help')
+help_parser.add_argument('help_subcommand', choices=sorted(cmds))
+for cmd, details in sorted(cmds.iteritems()):
+    add_command_help(subparser, subcommand_home, cmd, details)
 
 (args, cmd_args) = parser.parse_known_args()
 
-run_cmd = args.sub_command
+run_cmd = args.subcommand
 
-if args.sub_command == 'help':
-    run_cmd = args.help_sub_command
+if args.subcommand == 'help':
+    run_cmd = args.help_subcommand
     cmd_args = ['--help']
 
 def is_executable(path):
@@ -73,5 +71,4 @@
 if is_executable( subcommand_path ):
     os.execv(subcommand_path, [subcommand_path]+cmd_args)
 
-print [run_cmd]+cmd_args
 os.execvp('juju', ['juju',run_cmd]+cmd_args)

=== modified file 'sub-commands/aiki/cli.py'
--- sub-commands/aiki/cli.py	2012-06-18 21:55:21 +0000
+++ sub-commands/aiki/cli.py	2012-07-05 17:34:23 +0000
@@ -1,5 +1,8 @@
 import argparse
+import imp
 import logging
+import os
+import os.path
 import sys
 import traceback
 
@@ -20,8 +23,16 @@
     DEBUG=logging.DEBUG)
 
 
-def make_arg_parser(*args, **kwargs):
-    parser = argparse.ArgumentParser(*args, **kwargs)
+def make_arg_parser(root_parser, subcommand, **kwargs):
+    if root_parser is None:
+        if "help" in kwargs:
+            # Not valid for a top-level parser
+            del kwargs["help"]
+        parser = argparse.ArgumentParser(**kwargs)
+    else:
+        if "help" not in kwargs:
+            kwargs["help"] = kwargs["description"]  # Fallback
+        parser = root_parser.add_parser(subcommand, **kwargs)
     parser.add_argument(
         "-e", "--environment", default=None, help="Environment to act upon (otherwise uses default)", metavar="ENVIRONMENT")
     parser.add_argument(
@@ -106,3 +117,79 @@
         settings[k] = yaml.safe_load(v)
     remaining_args.extend(kv_iter)
     return settings, remaining_args
+
+
+def get_commands(dir):
+    """Given a directory, return valid jitsu commands"""
+    for cmd in os.listdir(dir):
+        # Filter anything not a bare command word (so dotted) and certain
+        # top-level names that can potentially look like commands
+        if '.' in cmd or cmd in ('Makefile', 'aiki'):
+            continue
+        yield (cmd,
+               dict(
+                home=dir,
+                symlink=os.path.islink(os.path.join(dir, cmd))))
+
+
+def add_command_help(subparser, subcommand_home, cmd, details):
+    """Given the subparser, adds a subcommand parser to it for use in help
+
+    Attempts two strategies:
+
+    1. If the script is Python (marked as such and parseable), the
+       script must support a top-level `make_parser` function, which
+       takes a subparser obj that help can be added to. This strategy
+       is only attempted if the command is built-in jitsu.
+
+    2. Otherwise, attempts to read the help from the initial comments
+       of the script.
+    """
+    filename = os.path.join(details["home"], cmd)
+    attempt_python_parse = subcommand_home == details["home"]
+    with open(filename) as f:
+        lines = f.readlines()
+    try:
+        if not attempt_python_parse or lines[0].rstrip() != "#!/usr/bin/env python":
+            raise ValueError("Not a Python source file")
+        source = "".join(lines)
+        mod_name = "jitsu_" + cmd
+        mod = imp.new_module(mod_name)
+        mod.__file__ = filename
+        mod.__name__ = mod_name
+        # NOTE: Does not set these attributes for mod: __loader__, __path__, __package__
+        exec source in mod.__dict__
+        mod.make_parser(subparser)
+    except Exception:
+        add_command_help_from_comments(subparser, cmd, details, lines)
+
+
+def add_command_help_from_comments(subparser, cmd, details, lines):
+    """Attempt parsing command script comments to get description and usage.
+
+    Uses the following convention. Symlinks are resolved:
+
+        #!/path/to/interpreter
+        # command - short description
+        # then any optional usage info as comments, 
+        # blank lines terminating, up to a
+        # Copyright notice
+    """
+    real_cmd = os.path.split(os.path.realpath(os.path.join(details["home"], cmd)))[1]
+    cmdspec = "# " + real_cmd + " - "  # Using real_cmd resolves to the symlinked version
+    help = cmd  # Fallback in case we don't see the cmdspec
+    usage = []
+    line_iter = iter(lines[1:])
+    for line in line_iter:
+        line = line.strip()
+        if cmdspec in line:
+            help = line.split(cmdspec)[1]
+            for usage_line in line_iter:
+                if usage_line.startswith("# ") and "Copyright" not in usage_line:
+                    usage.append(usage_line[2:])
+                else:
+                    break
+            break
+    subparser.add_parser(
+        cmd, description=cmd,
+        help=help, usage="".join(usage), formatter_class=argparse.RawDescriptionHelpFormatter)

=== modified file 'sub-commands/aiki/twistutils.py'
--- sub-commands/aiki/twistutils.py	2012-06-18 21:55:21 +0000
+++ sub-commands/aiki/twistutils.py	2012-07-05 17:34:23 +0000
@@ -1,4 +1,4 @@
-from twisted.internet.defer import Deferred
+from twisted.internet.defer import Deferred, DeferredList
 
 
 class CountDownLatch(object):
@@ -18,3 +18,16 @@
         if self.count == 0:
             if not self.completed.called:
                 self.completed.callback(None)  # Maybe collect together these results?
+
+
+def wait_for_results(deferreds, fireOnOneCallback):
+    d = DeferredList(deferreds, fireOnOneCallback=fireOnOneCallback, fireOnOneErrback=True, consumeErrors=True)
+    d.addCallback(lambda r: [x[1] for x in r])
+
+    def get_errors(f):
+        # Avoid spurious errors seen in closing connections; we don't care!
+        if not d.called:
+            return f.value.subFailure
+
+    d.addErrback(get_errors)
+    return d

=== modified file 'sub-commands/capfile'
--- sub-commands/capfile	2012-02-03 23:06:34 +0000
+++ sub-commands/capfile	2012-07-05 17:34:23 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 #
 # capfile - returns a capistrano formatted version of the juju status
 #

=== modified file 'sub-commands/deploy'
--- sub-commands/deploy	2012-04-23 06:22:32 +0000
+++ sub-commands/deploy	2012-07-05 17:34:23 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 
 # deploy - uses local repo like the charm store
 #
@@ -20,9 +20,8 @@
 import sys
 import os
 import os.path
-import yaml
 import logging
-import time
+
 
 logging.basicConfig(format='%(asctime)-15s %(name)s %(message)s', level=logging.INFO)
 logger = logging.getLogger('juju-jitsu')

=== modified file 'sub-commands/get-service-info'
--- sub-commands/get-service-info	2012-02-03 21:35:26 +0000
+++ sub-commands/get-service-info	2012-07-05 17:34:23 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 #
 # get-service-info - returns requested service info
 #

=== modified file 'sub-commands/get-unit-info'
--- sub-commands/get-unit-info	2012-02-03 20:34:28 +0000
+++ sub-commands/get-unit-info	2012-07-05 17:34:23 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 #
 # get-unit-info - returns requested unit info
 #

=== modified file 'sub-commands/gource'
--- sub-commands/gource	2012-04-25 05:31:50 +0000
+++ sub-commands/gource	2012-07-05 17:34:23 +0000
@@ -1,6 +1,6 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 #
-# gource - Turn juju status yaml into gource log
+# gource - converts juju status yaml output into gource log
 # Copyright (C) 2011 Canonical Ltd. All Rights Reserved.
 #
 # This program is free software: you can redistribute it and/or modify

=== modified file 'sub-commands/open-port'
--- sub-commands/open-port	2012-04-22 01:41:51 +0000
+++ sub-commands/open-port	2012-07-05 17:34:23 +0000
@@ -1,4 +1,6 @@
 #!/usr/bin/env python
+#
+# open-port - opens a port on a service unit or for all units in a service
 
 import argparse
 import zookeeper
@@ -11,7 +13,7 @@
 from twisted.internet import reactor
 from twisted.internet.defer import inlineCallbacks
 
-log = logging.getLogger("juitsu.port")
+log = logging.getLogger("jitsu.port")
 
 
 @inlineCallbacks

=== modified file 'sub-commands/run-as-hook'
--- sub-commands/run-as-hook	2012-05-30 22:16:44 +0000
+++ sub-commands/run-as-hook	2012-07-05 17:34:23 +0000
@@ -1,5 +1,6 @@
 #!/usr/bin/env python
-
+#
+# run-as-hook - runs a script or command as if it were a hook, on this computer
 """
 Examples:
 

=== modified file 'sub-commands/setup-environment'
--- sub-commands/setup-environment	2012-05-04 17:40:58 +0000
+++ sub-commands/setup-environment	2012-07-05 17:34:23 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 
 # setup-environment - sets up your juju environment interactively
 #

=== modified file 'sub-commands/topodump'
--- sub-commands/topodump	2012-05-09 19:24:49 +0000
+++ sub-commands/topodump	2012-07-05 17:34:23 +0000
@@ -1,4 +1,6 @@
 #!/usr/bin/env python
+#
+# topodump - dumps the zookeeper topology
 
 from twisted.internet.defer import inlineCallbacks
 from aiki.cli import make_arg_parser, setup_logging, run_command

=== modified file 'sub-commands/watch'
--- sub-commands/watch	2012-06-18 21:51:07 +0000
+++ sub-commands/watch	2012-07-05 17:34:23 +0000
@@ -4,11 +4,10 @@
 import logging
 import textwrap
 
-import yaml
-from twisted.internet.defer import inlineCallbacks, DeferredList, returnValue, succeed
+from twisted.internet.defer import inlineCallbacks, returnValue, succeed
 
 from aiki.cli import make_arg_parser, setup_logging, run_command, parse_relation_settings
-from aiki.twistutils import CountDownLatch
+from aiki.twistutils import CountDownLatch, wait_for_results
 from aiki.watcher import Watcher, ANY_SETTING
 from juju.hooks.cli import parse_port_protocol
 
@@ -17,34 +16,17 @@
 
 
 def main():
-    options = make_watch_parser()
+    parser = make_parser()
+    options = parse_options(parser)
     setup_logging(options)
     run_command(watch, options)
 
 
-def make_watch_parser():
-    condition_parser = argparse.ArgumentParser(description="Parses each condition")
-    condition_parser.add_argument(
-        "-n", "--num-units", default=None, type=int, metavar="NUM",
-        help="Number of units; only applies to services; defaults to 1 if using unit conditions")
-    condition_parser.add_argument("--open-port", default=[], dest="open_ports", action="append", metavar="PORT[/PROTOCOL]",
-                                  help="Open port for unit")
-    condition_parser.add_argument("--closed-port", default=[], dest="closed_ports", action="append", metavar="PORT[/PROTOCOL]",
-                                  help="Closed port for unit")
-    condition_parser.add_argument("-r", "--relation", default=None, metavar="RELATION", help="Specify relation")
-    condition_parser.add_argument("--setting", default=[], action="append", metavar="SETTING",
-                                  help="Relation setting exists for unit")
-    condition_parser.add_argument(
-        "--state", default=[], dest="states", action="append", metavar="STATE", help="State of unit")
-    condition_parser.add_argument(
-        "--x-state", default=[], dest="excluded_states", action="append", metavar="STATE", help="Cannot be in this state")
-    condition_parser.add_argument("rest", nargs=argparse.REMAINDER)
-
-    # If updating condition_parser, it can be useful to do
-    # condition_parser.format_help() to get the usage info for
-    # CONDITION; some manual editing will be required however.
-
+def make_parser(root_parser=None):
     main_parser = make_arg_parser(
+        root_parser, "watch",
+        help="waits on Juju environment for specified conditions",
+        description="Wait on Juju environment for specified conditions to become true",
         formatter_class=argparse.RawDescriptionHelpFormatter,
         usage=textwrap.dedent("""\
         watch [-h] [-e ENVIRONMENT]
@@ -95,7 +77,30 @@
     """))
     main_parser.add_argument("--any", default=False, action="store_true", help="Any of the conditions may be true")
     main_parser.add_argument("--number", default=False, action="store_true", help="Number output by the corresponding condition")
-
+    return main_parser
+
+
+def get_condition_parser():
+    # Note: generally when setting up a parser, the following would be
+    # added as a subparser. However, instead this is being used to
+    # parse a sequence of args, so it stands alone, due to the
+    # complexity of parsing conditions. Note that there is no help
+    # defined here, since this complexity requires that it be put in
+    # the main parser instead.
+    condition_parser = argparse.ArgumentParser()
+    condition_parser.add_argument("-n", "--num-units", default=None, type=int)
+    condition_parser.add_argument("--open-port", default=[], dest="open_ports", action="append")
+    condition_parser.add_argument("--closed-port", default=[], dest="closed_ports", action="append")
+    condition_parser.add_argument("-r", "--relation", default=None)
+    condition_parser.add_argument("--setting", default=[], action="append")
+    condition_parser.add_argument("--state", default=[], dest="states", action="append")
+    condition_parser.add_argument("--x-state", default=[], dest="excluded_states", action="append")
+    condition_parser.add_argument("rest", nargs=argparse.REMAINDER)
+    return condition_parser
+
+
+def parse_options(main_parser):
+    condition_parser = get_condition_parser()
     options, condition_args = main_parser.parse_known_args()
 
     # Partially parse (using remainder args) multiple times working
@@ -118,19 +123,6 @@
     return options
 
 
-def wait_for_results(deferreds, fireOnOneCallback):
-    d = DeferredList(deferreds, fireOnOneCallback=fireOnOneCallback, fireOnOneErrback=True, consumeErrors=True)
-    d.addCallback(lambda r: [x[1] for x in r])
-
-    def get_errors(f):
-        # Avoid spurious errors seen in closing connections; we don't care!
-        if not d.called:
-            return f.value.subFailure
-
-    d.addErrback(get_errors)
-    return d
-
-
 @inlineCallbacks
 def watch(result, client, options):
     wait_for_conditions = []