Mailing-List: contact commits-help@aurora.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@aurora.incubator.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: wfarner@apache.org
To: commits@aurora.incubator.apache.org
Message-Id: <4461c2d3c963435e9417f81d768b6451@git.apache.org>
Subject: incubator-aurora git commit: Simplify client help output and solely
 use argparse.
Date: Thu,  8 Jan 2015 21:14:21 +0000 (UTC)

Repository: incubator-aurora
Updated Branches:
  refs/heads/master 405392404 -> 44312974f


Simplify client help output and solely use argparse.

Bugs closed: AURORA-994

Reviewed at https://reviews.apache.org/r/29698/


Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/44312974
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/44312974
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/44312974

Branch: refs/heads/master
Commit: 44312974f2478ab71ceb1d196b82e9b880b3aab1
Parents: 4053924
Author: Bill Farner <wfarner@apache.org>
Authored: Thu Jan 8 13:13:37 2015 -0800
Committer: Bill Farner <wfarner@apache.org>
Committed: Thu Jan 8 13:13:37 2015 -0800

----------------------------------------------------------------------
 docs/client-commands.md                         |  13 +-
 docs/client.md                                  | 225 -------------------
 .../python/apache/aurora/client/cli/__init__.py | 101 +--------
 .../python/apache/aurora/client/cli/client.py   |   3 +-
 .../python/apache/aurora/client/cli/options.py  |  62 +----
 .../python/apache/aurora/client/cli/task.py     |  10 +-
 src/test/python/apache/aurora/client/cli/BUILD  |  12 -
 .../apache/aurora/client/cli/test_help.py       | 131 -----------
 .../apache/aurora/client/cli/test_plugins.py    |  25 +--
 .../python/apache/aurora/client/cli/util.py     |  21 +-
 10 files changed, 26 insertions(+), 577 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/44312974/docs/client-commands.md
----------------------------------------------------------------------
diff --git a/docs/client-commands.md b/docs/client-commands.md
index 75e6954..ae75263 100644
--- a/docs/client-commands.md
+++ b/docs/client-commands.md
@@ -1,12 +1,6 @@
 Aurora Client Commands
 ======================
 
-The most up-to-date reference is in the client itself: use the
-`aurora help` subcommand (for example, `aurora help` or
-`aurora help create`) to find the latest information on parameters and
-functionality. Note that `aurora help open` does not work, due to underlying issues with
-reflection.
-
 - [Introduction](#introduction)
 - [Cluster Configuration](#cluster-configuration)
 - [Job Keys](#job-keys)
@@ -279,10 +273,11 @@ if it contains hook definitions and activations that affect the
 Cron Jobs
 ---------
 
+You can manage cron jobs using the `aurora cron` command.  Please see
+[cron-jobs.md](cron-jobs.md) for more details.
+
 You will see various commands and options relating to cron jobs in
-`aurora -help` and similar. Ignore them, as they're not yet implemented.
-You might be able to use them without causing an error, but nothing happens
-if you do.
+`aurora -h` and similar. Ignore them, as they're not yet implemented.
 
 Comparing Jobs
 --------------

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/44312974/docs/client.md
----------------------------------------------------------------------
diff --git a/docs/client.md b/docs/client.md
deleted file mode 100644
index 3ec39b4..0000000
--- a/docs/client.md
+++ /dev/null
@@ -1,225 +0,0 @@
-Aurora Client
-=============
-
-Goals
--------
-
-* A command line tool for interacting with Aurora that is easy for
-  users to understand.
-* A noun/verb command model.
-* A modular source-code architecture.
-* Non-disruptive transition for users.
-
-Design
-------
-
-### Interface
-
-In this section, we'll walk through the types of objects that the
-client can manipulate, and the operations that need to be provided for
-each object. These form the primary interface that engineers will use
-to interact with Aurora.
-
-In the command-line, each of the object types will have an Aurora
-subcommand. The commands to manipulate the object type will follow the
-type.
-
-### The Job Noun
-
-A job is a configured program ready to run in Aurora. A job is,
-conceptually, a task factory: when a job is submitted to the Aurora
-scheduler, it creates a collection of tasks. The job contains a
-complete description of everything it needs to create a collection of
-tasks. (Note that this subsumes "service" commands. A service is just
-a task whose configuration sets the is_service flag, so we don't have
-separate commands for working with services.) Jobs are specified using
-`cluster/role/env/name` jobkey syntax.
-
-* `aurora job create *jobkey* *config*`:  submits a job to a cluster, launching
-  the task(s) specified by the job config.
-* `aurora job status *jobkey*`: query job status. Prints information about the
-  job, whether it's running, etc., to standard out. If jobkey includes globs,
-  it should list all jobs that match the glob
-* `aurora job kill *jobkey*/*instanceids*`: kill/stop some of a jobs instances.
-  This stops a job' tasks; if the job has service tasks, they'll be  disabled,
-  so that they won't restart.
-* `aurora job killall *jobkey*`: kill all of the instances of a job. This
-  is distinct from the *kill* command as a safety measure: omitting the
-  instances from a kill command shouldn't result in destroying the entire job.
-* `aurora job restart *jobkey*`: conceptually, this will kill a job, and then
-  launch it again. If the job does not exist, then fail with an error
-  message.  In fact, the underlying implementation does the
-  kill/relaunch on a rolling basis - so it's not an immediate kill of
-  all shards/instances, followed by a delay as all instances relaunch,
-  but rather a controlled gradual process.
-* `aurora job list *jobkey*`: list all jobs that match the jobkey spec that are
-  registered with the scheduler. This will include both jobs that are
-  currently running, and jobs that are scheduled to run at a later
-  time. The job key can be partial: if it specifies cluster, all jobs
-  on the cluster will be listed; cluster/role, all jobs running on the cluster
-  under the role will be listed, etc.
-
-The Schedule Noun (Cron)
---------------------------
-
-Cron is a scheduler adjunct that periodically runs a job on a
-schedule. The cron commands all manipulate cron schedule entries. The
-schedules are specified as a part of the job configuration.
-
-* `aurora cron schedule jobkey config`: schedule a job to run by cron. If a cron
-  job already exists replaces its template with a new one.
-* `aurora cron deschedule jobkey`: removes a jobs entry from the cron schedule.
-* `aurora cron status jobkey`: query for a scheduled job's status.
-
-The Quota Noun
----------------
-
-A quota is a data object maintained by the scheduler that specifies the maximum
-resources that may be consumed by jobs owned by a particular role. In the future,
-we may add new quota types. At some point, we'll also probably add an administrators
-command to set quotas.
-
-* `aurora quota get *cluster/role*`
-
-
-### Command Structure and Options Processing
-
-The implementation will follow closely on Pants goals. Pants goals use
-a static registration system to add new subcommands. In pants, each
-goal command is an implementation of a command interface, and provides
-implementations of methods to register options and parameters, and to
-actually execute the command. In this design, commands are modular and
-easy to implement, debug, and combine in different ways.
-
-For the Aurora client, we plan to use a two-level variation of the
-basic concept from pants. At the top-level we will have nouns. A noun
-will define some common command-line parameters required by all of its
-verbs, and will provide a registration hook for attaching verbs. Nouns
-will be implemented as a subclass of a basic Noun type.
-
-Each verb will, similarly, be implemented as a subclass of Verb. Verbs
-will be able to specify command-line options and parameters.
-
-Both `Noun` and `Verb` will be subclasses of a common base-class `AuroraCommand`:
-
-    class AuroraCommand(object):
-      def get_options(self):
-      """Gets the set of command-line options objects for this command.
-      The result is a list of CommandOption objects.
-       """
-        pass
-
-      @property
-      def help(self):
-        """Returns the help message for this command"""
-
-      @property
-      def usage(self):
-        """Returns a short usage description of the command"""
-
-      @property
-      def name(self):
-        """Returns the command name"""
-
-
-A command-line tool will be implemented as an instance of a `CommandLine`:
-
-    class CommandLine(object):
-      """The top-level object implementing a command-line application."""
-
-      @property
-      def name(self):
-        """Returns the name of this command-line tool"""
-
-      def print_out(self, str):
-        print(str)
-
-      def print_err(self, str):
-        print(str, file=sys.stderr)
-
-      def register_noun(self, noun):
-        """Adds a noun to the application"""
-
-      def register_plugin(self, plugin):
-	     """Adds a configuration plugin to the system"""
-
-
-Nouns are registered into a command-line using the `register_noun`
-method. They are weakly coupled to the application, making it easy to
-use a single noun in several different command-line tools. Nouns allow
-the registration of verbs using the `register_verb` method.
-
-When commands execute, they're given an instance of a *context object*.
-The context object must be an instance of a subclass of `AuroraCommandContext`.
-Options, parameters, and IO are all accessed using the context object. The context
-is created dynamically by the noun object owning the verb being executed. Developers
-are strongly encouraged to implement custom contexts for their nouns, and move functionality
-shared by the noun's verbs into the context object. The context interface is:
-
-    class Context(object):
-      class Error(Exception): pass
-
-      class ArgumentException(Error): pass
-
-      class CommandError(Error):
-
-      @classmethod
-      def exit(cls, code, msg):
-	    """Exit the application with an error message"""
-        raise cls.CommandError(code, msg)
-
-     def print_out(self, msg, indent=0):
-       """Prints a message to standard out, with an indent"""
-
-     def print_err(self, msg, indent=0):
-       """Prints a message to standard err, with an indent"""
-
-
-In addition to nouns and verbs, there's one more kind of registerable
-component, called a *configuration plugin*. These objects add a set of
-command-line options that can be passed to *all* of the commands
-implemented in the tool. Before the command is executed, the
-configuration plugin will be invoked, and will process its
-command-line arguments. This is useful for general configuration
-changes, like establish a secure tunnel to talk to machines in a
-datacenter. (A useful way to think of a plugin is as something like an
-aspect that can be woven in to aurora to provide environment-specific
-configuration.) A configuration plugin is implemented as an instance
-of class `ConfigurationPlugin`, and registered with the
-`register_plugin` method of the `CommandLine` object. The interface of
-a plugin is:
-
-    class ConfigurationPlugin(object):
-      """A component that can be plugged in to a command-line."""
-
-      @abstractmethod
-      def get_options(self):
-        """Return the set of options processed by this plugin"""
-
-      @abstractmethod
-      def execute(self, context):
-        """Run the context/command line initialization code for this plugin."""
-
-
-### Command Execution
-
-The options process and command execution is built as a facade over Python's
-standard argparse. All of the actual argument processing is done by the
-argparse library.
-
-Once the options are processed, the framework will start to execute the command.
-Command execution consists of:
-
-# Create a context object. The framework will use the argparse options to identify
-  which noun is being invoked, and will call that noun's `create_context` method.
-  The argparse options object will be stored in the context.
-# Execute any configuration plugins. Before any command is invoked, the framework
-  will first iterate over all of the registered configuration plugins. For each
-  plugin, it will invoke the `execute` method.
-# The noun will use the context to find out what verb is being invoked, and it will
-  then call that verb's `execute` method.
-# The command will exit. Its return code will be whatever was returned by the verb's
-  `execute` method.
-
-Commands are expected to return a code from a list of standard exit codes,
-which can be found in `src/main/python/apache/aurora/client/cli/__init__.py`.

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/44312974/src/main/python/apache/aurora/client/cli/__init__.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/__init__.py b/src/main/python/apache/aurora/client/cli/__init__.py
index 395819f..5060577 100644
--- a/src/main/python/apache/aurora/client/cli/__init__.py
+++ b/src/main/python/apache/aurora/client/cli/__init__.py
@@ -186,10 +186,6 @@ class AuroraCommand(AbstractClass):
     """Returns the help message for this command"""
 
   @abstractproperty
-  def usage(self):
-    """Returns a short usage description of the command"""
-
-  @abstractproperty
   def name(self):
     """Returns the command name"""
 
@@ -229,56 +225,11 @@ class CommandLine(AbstractClass):
   def setup_options_parser(self):
     """ Builds the options parsing for the application."""
     self.parser = argparse.ArgumentParser()
-    subparser = self.parser.add_subparsers(dest="noun")
-    for (name, noun) in self.nouns.items():
+    subparser = self.parser.add_subparsers(dest="noun", title='commands')
+    for name, noun in self.nouns.items():
       noun_parser = subparser.add_parser(name, help=noun.help)
       noun.internal_setup_options_parser(noun_parser)
 
-  def help_cmd(self, args):
-    """Generates a help message for a help request.
-    There are three kinds of help requests: a simple no-parameter request (help) which generates
-    a list of all of the commands; a one-parameter (help noun) request, which generates the help
-    for a particular noun, and a two-parameter request (help noun verb) which generates the help
-    for a particular verb.
-    """
-    if args is None or len(args) == 0:
-      self.print_out(self.composed_help)
-    elif len(args) == 1:
-      if args[0] in self.nouns:
-        self.print_out(self.nouns[args[0]].composed_help)
-        return EXIT_OK
-      else:
-        self.print_err('Unknown noun "%s"' % args[0])
-        self.print_err("Valid nouns are: %s" % [k for k in self.nouns])
-        return EXIT_INVALID_PARAMETER
-    elif len(args) == 2:
-      if args[0] in self.nouns:
-        if args[1] in self.nouns[args[0]].verbs:
-          self.print_out(self.nouns[args[0]].verbs[args[1]].composed_help)
-          return EXIT_OK
-        else:
-          self.print_err('Noun "%s" does not support a verb "%s"' % (args[0], args[1]))
-          verbs = [v for v in self.nouns[args[0]].verbs]
-          self.print_err('Valid verbs for "%s" are: %s' % (args[0], verbs))
-          return EXIT_INVALID_PARAMETER
-      else:
-        self.print_err("Unknown noun %s" % args[0])
-        return EXIT_INVALID_PARAMETER
-    else:
-      self.print_err("Unknown help command: %s" % (" ".join(args)))
-      self.print_err(self.composed_help)
-      return EXIT_INVALID_PARAMETER
-
-  @property
-  def composed_help(self):
-    """Get a fully composed, well-formatted help message"""
-    result = ["Aurora Client version %s" % get_client_version(), "Usage:"]
-    for noun in self.registered_nouns:
-      result += ["==Commands for %ss" % noun]
-      result += ["  %s" % s for s in self.nouns[noun].usage] + [""]
-    result.append("\nRun 'help noun' or 'help noun verb' for help about a specific command")
-    return "\n".join(result)
-
   @abstractmethod
   def register_nouns(self):
     """This method should overridden by applications to register the collection of nouns
@@ -357,8 +308,6 @@ class CommandLine(AbstractClass):
     except ConfigurationPlugin.Error as e:
       print("Error in configuration plugin before dispatch: %s" % e.msg, file=sys.stderr)
       return e.code
-    if args[0] == "help":
-      return self.help_cmd(args[1:])
     noun, context = self._parse_args(args)
     logging.debug("Command=(%s)", args)
     pre_result = self._run_pre_hooks_and_plugins(context, args)
@@ -366,7 +315,7 @@ class CommandLine(AbstractClass):
       return pre_result
     try:
       result = noun.execute(context)
-      assert result is not None, "Noun return value is None!"
+      assert result is not None, "Command return value is None!"
       if result == EXIT_OK:
         logging.debug("Command terminated successfully")
         GlobalCommandHookRegistry.run_post_hooks(context, context.options.noun,
@@ -421,8 +370,8 @@ class Noun(AuroraCommand):
     into a python argparse subparser for this noun.
     """
     self.setup_options_parser(argparser)
-    subparser = argparser.add_subparsers(dest="verb")
-    for (name, verb) in self.verbs.items():
+    subparser = argparser.add_subparsers(dest="verb", title='subcommands')
+    for name, verb in self.verbs.items():
       vparser = subparser.add_parser(name, help=verb.help)
       for opt in verb.get_options():
         opt.add_to_parser(vparser)
@@ -432,10 +381,6 @@ class Noun(AuroraCommand):
       for opt in GlobalCommandHookRegistry.get_options():
         opt.add_to_parser(vparser)
 
-  @property
-  def usage(self):
-    return ["%s %s" % (self.name, self.verbs[verb].usage) for verb in self.verbs]
-
   @classmethod
   def create_context(cls):
     """Commands access state through a context object. The noun specifies what kind
@@ -443,16 +388,9 @@ class Noun(AuroraCommand):
     """
     pass
 
-  @property
-  def composed_help(self):
-    result = ['Usage for noun "%s":' % self.name]
-    result += ["    %s %s" % (self.name, self.verbs[verb].usage) for verb in self.verbs]
-    result += [self.help]
-    return "\n\n".join(result)
-
   def execute(self, context):
     if context.options.verb not in self.verbs:
-      raise self.InvalidVerbException("Noun %s does not have a verb %s" %
+      raise self.InvalidVerbException("Command %s does not have subcommand %s" %
           (self.name, context.options.verb))
     return self.verbs[context.options.verb].execute(context)
 
@@ -464,37 +402,10 @@ class Verb(AuroraCommand):
     """Create a link from a verb to its noun."""
     self.noun = noun
 
-  @property
-  def usage(self):
-    """Get a brief usage-description for the command.
-    A default usage string is automatically generated, but for commands with many options,
-    users may want to specify usage themselves.
-    """
-    result = [self.name]
-    for plugin in self.noun.commandline.plugins:
-      result += ["  " + opt.render_usage() for opt in plugin.get_options()]
-    result += [opt.render_usage() for opt in self.get_options()]
-
-    return " ".join(result)
-
   @abstractmethod
   def get_options(self):
     pass
 
-  @property
-  def composed_help(self):
-    """Generate the composed help message shown when the user requests help about this verb"""
-    result = ['Usage for verb "%s %s":' % (self.noun.name, self.name)]
-    result += ["  " + self.usage]
-    result += ["Options:"]
-    for opt in self.get_options():
-      result += ["  " + s for s in opt.render_help()]
-    for plugin in self.noun.commandline.plugins:
-      for opt in plugin.get_options():
-        result += ["  " + s for s in opt.render_help()]
-    result += ["", self.help]
-    return "\n".join(result)
-
   @abstractmethod
   def execute(self, context):
     pass

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/44312974/src/main/python/apache/aurora/client/cli/client.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/client.py b/src/main/python/apache/aurora/client/cli/client.py
index 939e32b..24516d1 100644
--- a/src/main/python/apache/aurora/client/cli/client.py
+++ b/src/main/python/apache/aurora/client/cli/client.py
@@ -89,8 +89,9 @@ class AuroraCommandLine(CommandLine):
 
 def proxy_main():
   client = AuroraCommandLine()
+  # Defaulting to '-h' results in a similar, but more inviting message than 'too few arguments'.
   if len(sys.argv) == 1:
-    sys.argv.append("help")
+    sys.argv.append('-h')
   sys.exit(client.execute(sys.argv[1:]))
 
 if __name__ == '__main__':

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/44312974/src/main/python/apache/aurora/client/cli/options.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/options.py b/src/main/python/apache/aurora/client/cli/options.py
index b7f5a03..1106efa 100644
--- a/src/main/python/apache/aurora/client/cli/options.py
+++ b/src/main/python/apache/aurora/client/cli/options.py
@@ -15,12 +15,12 @@
 from argparse import ArgumentTypeError
 from collections import namedtuple
 
-from twitter.common.lang import Compatibility
 from twitter.common.quantity.parse_simple import parse_time
 
 from apache.aurora.common.aurora_job_key import AuroraJobKey
 
 
+# TODO(wfarner): Consider removing, it doesn't appear this wrapper is useful.
 class CommandOption(object):
   """A lightweight encapsulation of an argparse option specification"""
 
@@ -39,66 +39,6 @@ class CommandOption(object):
     self.type = self.kwargs.get('type')
     self.help = self.kwargs.get('help', '')
 
-  def is_mandatory(self):
-    return self.kwargs.get('required', not self.name.startswith('--'))
-
-  def get_displayname(self):
-    """Get a display name for a the expected format of a parameter value"""
-    if 'metavar' in self.kwargs:
-      displayname = self.kwargs['metavar']
-    elif self.type is str:
-      displayname = "str"
-    elif isinstance(self.type, Compatibility.string):
-      displayname = self.type
-    elif isinstance(self.type, Compatibility.integer):
-      displayname = "int",
-    elif self.name.startswith('--'):
-      displayname = self.name[2:]
-    else:
-      displayname = self.name
-    return displayname
-
-  def get_destination(self):
-    """Get the attribute name this option will be stored under internally."""
-    if self.kwargs.get('dest'):
-      return self.kwargs['dest']
-    # See the spec here: https://docs.python.org/2/library/argparse.html#dest
-    return self.name.lstrip('--').replace('-', '_')
-
-  def get_default_value(self):
-    """Get the default value if no argument for this option is supplied."""
-    return self.kwargs.get('default')
-
-  def render_usage(self):
-    """Create a usage string for this option"""
-    if not self.name.startswith('--'):
-      return self.get_displayname()
-    if "action" in self.kwargs:
-      if self.kwargs["action"] == "store_true":
-        return "[%s]" % self.name
-      elif self.kwargs["action"] == "store_false":
-        return "[--no-%s]" % self.name[2:]
-    if self.type is None and "choices" in self.kwargs:
-      return "[%s=%s]" % (self.name, self.kwargs["choices"])
-    else:
-      return "[%s=%s]" % (self.name, self.get_displayname())
-
-  def render_help(self):
-    """Render a full help message for this option"""
-    result = ""
-    if "action" in self.kwargs and self.kwargs["action"] == "store_true":
-      result = self.name
-    elif "action" in self.kwargs and self.kwargs["action"] == "store_false":
-      result = "--no-%s" % self.name[2:]
-    elif self.type is None and "choices" in self.kwargs:
-      result = "%s=%s" % (self.name, self.kwargs["choices"])
-    else:
-      if self.name.startswith("--"):
-        result = "%s=%s" % (self.name, self.get_displayname())
-      else:
-        result = "%s" % self.get_displayname()
-    return [result, "\t" + self.help]
-
   def add_to_parser(self, parser):
     """Add this option to an option parser"""
     parser.add_argument(*self.args, **self.kwargs)

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/44312974/src/main/python/apache/aurora/client/cli/task.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/task.py b/src/main/python/apache/aurora/client/cli/task.py
index e084c5b..2154898 100644
--- a/src/main/python/apache/aurora/client/cli/task.py
+++ b/src/main/python/apache/aurora/client/cli/task.py
@@ -43,13 +43,9 @@ class RunCommand(Verb):
 
   @property
   def help(self):
-    return """Usage: aurora task run cluster/role/env/job cmd
-
-  Runs a shell command on machines currently hosting instances of a single job.
-
+    return """runs a shell command on machines currently hosting instances of a single job.
   This feature supports the same command line wildcards that are used to
   populate a job's commands.
-
   This means anything in the {{mesos.*}} and {{thermos.*}} namespaces.
   """
 
@@ -80,9 +76,7 @@ class SshCommand(Verb):
 
   @property
   def help(self):
-    return """usage: aurora task ssh cluster/role/env/job/instance [args...]
-
-  Initiate an SSH session on the machine that a task instance is running on.
+    return """initiates an SSH session on the machine that a task instance is running on.
   """
 
   def get_options(self):

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/44312974/src/test/python/apache/aurora/client/cli/BUILD
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/BUILD b/src/test/python/apache/aurora/client/cli/BUILD
index bbac5c8..407cda9 100644
--- a/src/test/python/apache/aurora/client/cli/BUILD
+++ b/src/test/python/apache/aurora/client/cli/BUILD
@@ -19,7 +19,6 @@ python_test_suite(
     ':client',
     ':command_hooks',
     ':cron',
-    ':help',
     ':inspect',
     ':job',
     ':config',
@@ -66,17 +65,6 @@ python_tests(
 )
 
 python_tests(
-  name = 'help',
-  sources = [ 'test_help.py' ],
-  dependencies = [
-    '3rdparty/python:mock',
-    '3rdparty/python:twitter.common.contextutil',
-    'src/main/python/apache/aurora/client/cli',
-    'src/main/python/apache/aurora/client/cli:client_lib',
-  ]
-)
-
-python_tests(
   name = 'command_hooks',
   sources = [ 'test_command_hooks.py' ],
   dependencies = [

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/44312974/src/test/python/apache/aurora/client/cli/test_help.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_help.py b/src/test/python/apache/aurora/client/cli/test_help.py
deleted file mode 100644
index 9fa05e6..0000000
--- a/src/test/python/apache/aurora/client/cli/test_help.py
+++ /dev/null
@@ -1,131 +0,0 @@
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-import contextlib
-import unittest
-
-from mock import patch
-
-from apache.aurora.client.cli import EXIT_INVALID_PARAMETER, EXIT_OK
-from apache.aurora.client.cli.client import AuroraCommandLine
-
-
-class TestHelp(unittest.TestCase):
-  """Tests of the help command for the Aurora client framework"""
-
-  def setUp(self):
-    self.cmd = AuroraCommandLine()
-    self.transcript = []
-    self.err_transcript = []
-
-  def mock_print(self, output, indent=0):
-    self.transcript.extend(output.splitlines())
-
-  def mock_print_err(self, output, indent=0):
-    self.err_transcript.extend(output.splitlines())
-
-  @contextlib.contextmanager
-  def standard_mocks(self):
-    with contextlib.nested(
-        patch('apache.aurora.client.cli.client.AuroraCommandLine.print_out',
-            side_effect=self.mock_print),
-        patch('apache.aurora.client.cli.client.AuroraCommandLine.print_err',
-            side_effect=self.mock_print_err),
-        patch('apache.aurora.client.cli.get_client_version', return_value='0.0.test')):
-
-      yield
-
-  def test_all_help(self):
-    for noun in self.cmd.registered_nouns:
-      with self.standard_mocks():
-        self.cmd.execute(['help', noun])
-        assert 'Usage for noun "%s":' % noun in self.transcript
-        assert self.err_transcript == []
-        self.transcript = []
-        for verb in self.cmd.nouns.get(noun).verbs.keys():
-          self.cmd.execute(['help', noun, verb])
-          assert 'Usage for verb "%s %s":' % (noun, verb) in self.transcript
-          assert self.err_transcript == []
-          self.transcript = []
-
-  def test_help(self):
-    with self.standard_mocks():
-      self.cmd.execute(['help'])
-      assert len(self.transcript) > 10
-      assert self.transcript[0] == 'Aurora Client version 0.0.test'
-      assert self.transcript[1] == 'Usage:'
-      assert '==Commands for jobs' in self.transcript
-      assert '==Commands for quotas' in self.transcript
-
-  def test_usage_string_includes_plugin_options(self):
-    plugin_options = []
-    for plugin in self.cmd.plugins:
-      if plugin.get_options() is not None:
-        plugin_options += [p for p in plugin.get_options()]
-    with self.standard_mocks():
-      for noun in self.cmd.registered_nouns:
-        for verb in self.cmd.nouns.get(noun).verbs.keys():
-          self.transcript = []
-          self.cmd.execute(['help', noun, verb])
-          for opt in plugin_options:
-            assert any(opt.name in line for line in self.transcript)
-
-  def test_command_help_does_not_have_unset_str_metavars(self):
-    plugin_options = []
-    for plugin in self.cmd.plugins:
-      if plugin.get_options() is not None:
-        plugin_options += [p for p in plugin.get_options()]
-    with self.standard_mocks():
-      for noun in self.cmd.registered_nouns:
-        for verb in self.cmd.nouns.get(noun).verbs.keys():
-          self.transcript = []
-          self.cmd.execute(['help', noun, verb])
-          print(self.transcript)
-          for opt in plugin_options:
-            assert not any(line.endswith('=str') for line in self.transcript)
-
-  def test_help_noun(self):
-    with self.standard_mocks():
-      self.cmd.execute(['help', 'job'])
-      assert len(self.transcript) > 10
-      assert self.transcript[0] == 'Usage for noun "job":' in self.transcript
-      assert not any('quota' in t for t in self.transcript)
-      assert any('job status' in t for t in self.transcript)
-      assert any('job list' in t for t in self.transcript)
-
-  def test_help_verb(self):
-    with self.standard_mocks():
-      assert self.cmd.execute(['help', 'job', 'status']) == EXIT_OK
-      assert len(self.transcript) > 5
-      assert self.transcript[0] == 'Usage for verb "job status":' in self.transcript
-      assert not any('quota' in t for t in self.transcript)
-      assert not any('list' in t for t in self.transcript)
-      assert "Options:" in self.transcript
-      assert any('status' for t in self.transcript)
-
-  def test_help_unknown_noun(self):
-    with self.standard_mocks():
-      assert self.cmd.execute(['help', 'nothing']) == EXIT_INVALID_PARAMETER
-      assert len(self.transcript) == 0
-      assert len(self.err_transcript) == 2
-      assert 'Unknown noun "nothing"' == self.err_transcript[0]
-      assert "Valid nouns" in self.err_transcript[1]
-
-  def test_help_unknown_verb(self):
-    with self.standard_mocks():
-      assert self.cmd.execute(['help', 'job', 'nothing']) == EXIT_INVALID_PARAMETER
-      assert len(self.transcript) == 0
-      assert len(self.err_transcript) == 2
-      assert 'Noun "job" does not support a verb "nothing"' == self.err_transcript[0]
-      assert 'Valid verbs for "job" are' in self.err_transcript[1]

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/44312974/src/test/python/apache/aurora/client/cli/test_plugins.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_plugins.py b/src/test/python/apache/aurora/client/cli/test_plugins.py
index cf742a3..aa45851 100644
--- a/src/test/python/apache/aurora/client/cli/test_plugins.py
+++ b/src/test/python/apache/aurora/client/cli/test_plugins.py
@@ -15,7 +15,7 @@
 from mock import create_autospec, patch
 from twitter.common.contextutil import temporary_file
 
-from apache.aurora.client.cli import ConfigurationPlugin, EXIT_OK
+from apache.aurora.client.cli import ConfigurationPlugin
 from apache.aurora.client.cli.client import AuroraCommandLine
 from apache.aurora.client.cli.options import CommandOption
 from apache.aurora.config import AuroraConfig
@@ -142,26 +142,3 @@ class TestPlugins(AuroraClientCommandTest):
       # Check that the plugin did its job.
       assert mock_context.bogosity == "maximum"
       assert mock_context.after
-
-  def mock_print(self, str):
-    for str in str.split('\n'):
-      self.transcript.append(str)
-
-  def mock_print_err(self, str):
-    for str in str.split('\n'):
-      self.err_transcript.append(str)
-
-  def test_plugin_options_in_help(self):
-    cmd = AuroraCommandLine()
-    cmd.register_plugin(BogusPlugin())
-    self.transcript = []
-    self.err_transcript = []
-    with patch('apache.aurora.client.cli.client.AuroraCommandLine.print_out',
-        side_effect=self.mock_print):
-      assert cmd.execute(['help', 'job', 'status']) == EXIT_OK
-      assert len(self.transcript) > 5
-      assert self.transcript[0] == 'Usage for verb "job status":' in self.transcript
-      assert not any('quota' in t for t in self.transcript)
-      assert not any('list' in t for t in self.transcript)
-      assert "Options:" in self.transcript
-      assert any('bogosity' in t for t in self.transcript)

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/44312974/src/test/python/apache/aurora/client/cli/util.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/util.py b/src/test/python/apache/aurora/client/cli/util.py
index 1fa1207..147d418 100644
--- a/src/test/python/apache/aurora/client/cli/util.py
+++ b/src/test/python/apache/aurora/client/cli/util.py
@@ -44,17 +44,16 @@ from gen.apache.aurora.api.ttypes import (
 
 
 def mock_verb_options(verb):
-  return create_options_mock(verb.get_options())
-
-
-def create_options_mock(options):
-  """Inspects a list of CommandOption instances for their attribute and default values and
-     constructs a mock with specification from it."""
-  attributes = [o.get_destination() for o in options]
-  mock = Mock(spec_set=attributes)
-  for o in options:
-    setattr(mock, o.get_destination(), o.get_default_value())
-  return mock
+  # Handle default values opt.kwargs.get('default')
+  def opt_name(opt):
+    return opt.name.lstrip('--').replace('-', '_')
+
+  options = Mock(spec_set=[opt_name(opt) for opt in verb.get_options()])
+  # Apply default values to options.
+  for opt in verb.get_options():
+    if 'default' in opt.kwargs:
+      setattr(options, opt_name(opt), opt.kwargs.get('default'))
+  return options
 
 
 class FakeAuroraCommandLine(AuroraCommandLine):