Mailing-List: contact dev-help@aurora.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@aurora.apache.org
MIME-Version: 1.0
Date: Thu, 7 Jan 2016 17:18:05 -0800
Message-ID: 
 <CAFWq12VJPOxDViDP5OsmXU3skHR6rrp5Zp+Xgi8HgeHic6pTJw@mail.gmail.com>
Subject: [PROPOSAL] Replace commons-args
From: Bill Farner <wfarner@apache.org>
To: dev@aurora.apache.org
Content-Type: multipart/alternative; boundary=001a11c1d4f2fc14790528c85bee

--001a11c1d4f2fc14790528c85bee
Content-Type: text/plain; charset=UTF-8

All,

I propose that we replace our current command line args system.  For those
unaware, the args system [1] we currently use was forked out of Twitter
Commons several months ago.  The goal when forking was to adapt/maintain or
eventually replace these libraries.  Given the code volume and complexity
of commons-args, i propose we replace it and address some issues along the
way.

I suggest several goals:
a. sidestep current development hurdles we have encountered with the args
system (intellij/gradle not working nicely with apt)
b. encourage better testability of Module classes by always injecting all
args
c. leverage a well-maintained third-party argument parsing library
d. stretch: enable user-friendly features like logical option groups for
better help/usage output
e. stretch: enable alternative configuration inputs like a configuration
file or environment variables

(b) is currently an issue because command line arguments are driven from
pseudo-constants within the code, for example:

    @NotNull
    @CmdLine(name = "cluster_name", help = "Name to identify the cluster
being served.")
    private static final Arg<String> CLUSTER_NAME = Arg.create();

    @NotNull
    @NotEmpty
    @CmdLine(name = "serverset_path", help = "ZooKeeper ServerSet path to
register at.")
    private static final Arg<String> SERVERSET_PATH = Arg.create();

This makes it simple to add command line arguments.  However, it means that
a level of indirection is needed to parameterize and test the code
consuming arg values.  We have various examples of this throughout the
project.

I propose that we change the way command line arguments are declared such
that a Module with the above arguments would instead declare an interface
that produces its parameters:

    public interface Params {
      @Help("Name to identify the cluster being served.")
      String clusterName();

      @NotEmpty
      @Help("ZooKeeper ServerSet path to register at.")
      String serversetPath();
    }

    public SchedulerModule(Params params) {
      // Params are supplied to the module constructor.
    }

Please see this review for a complete example of this part of the change:
https://reviews.apache.org/r/41804

This is roughly the same amount of overhead for declaring arguments as the
current scenario, with the addition of a very obvious mechanism for
swapping the source of parameters.  This allows us to isolate the body of
code responsible for supplying configuration values, which we lack today.

The remaining work is to bridge the gap between a command line argument
system and Parameter interfaces.  This is relatively easy to do with
dynamic proxies.  I have posted a proof of concept here:
https://reviews.apache.org/r/42042

Regarding (c), i have done some analysis of libraries available and i
suggest argparse4j [3].  It has thorough documentation, no transitive
dependencies, and is being actively developed (last release Dec 2015).
However, i would like to emphasize that i think we should minimize coupling
to the argument parsing library so that we may switch in the future.
Argparse4j has a feature that makes the non-critical feature (d) possible.

With that, what do you think?  Are there other goals we should add?  Does
the plan make sense?

[1]
https://github.com/apache/aurora/tree/master/commons-args/src/main/java/org/apache/aurora/common/args
[2] https://github.com/twitter/commons
[3] https://argparse4j.github.io/

--001a11c1d4f2fc14790528c85bee--