The proposal to merge lp:~frankban/lpsetup/out-of-control into lp:lpsetup has
been updated.
Description changed to:
= Summary =
This branch is the result of a previous experimental branch. The goal is to
reduce the inversion of control taking place in the argparser/subcommands
machinery.
Before:
We had a customized *ArgumentParser* being able to register subcommands.
Subcommands were subclasses of *BaseSubCommand*, registered using
*ArgumentParser.register_subcommand*. From that point on, the subcommands
framework took care of the process of validating, restarting as root and
actual running each sub command.
In this branch:
The *ArgumentParser* is more or less a default *argparse.ArgumentParser*.
The only change there is a method override done to store actions in
a "public" attribute. Subcommands are just objects implementing a
pre-defined
protocol, that is described in the branch. So subcommands can be instances,
or just modules.
The process of registering and running a subcommand is now handled by
functions
in `cli.py`, and the code flow should be easier to follow because several
actions are now executed in a procedural way: prepare the parser, add
common
arguments, add user-interaction related arguments, restart as root,
namespace
initialization and validation, ...
However, the inversion of control is still present, but it's moved forward
to
the point of executing actual subcommand stuff: i.e. when *subcommand.run*
is
called. As a consequence, while the subcommand base classes are still
there,
they are now more lightweight, and they represent only a flexible and easy
way
to implement the subcommand protocol. They still introduce and handle the
concepts of handlers and steps, which are more well-documented in the code.
If we will decide in the future to follow the path of further reducing the
inversion
of control (e.g. getting rid of subcommands base classes), this branch could be
considered as an incremental step.
For now, my opinion is that this could be a good trade off between the need for
simplicity
and the generality/"DRYness" of OOP.
This branch adds documentation and massively reorganizes code: methods are now
functions, tests have been changed accordingly, some unused modules have been
removed,
etc... so the diff is HUGE, and I am very sorry.
The integration test `test_install_lxc.py` passes.
== Other changes ==
Introduced a basic documentation for subcommands.
Modified *ArgumentParser* and base subcommand classes as described above. Also
removed no longer valid doctests. The code exercised by removed doctests is
fully
covered by unittests.
*subcommand.get_description(namespace) is now called to display subcommands'
descriptions in a generic way: steps based subcommands override that method so
that
the resulting description is created collecting steps' descriptions.
The `argparser` module now defines several functions explicitly called by cli:
*add_common_arguments*, *get_args_from_namespace*, *init_namespace*,
*restart_as_root*.
The `cli `module itself now contains some helper functions:
- *get_parse*r returns an argument parser with all subcommands registered
- *prepare_parser* takes a parser (or a subparser) and a subcommand, adds
common
arguments to the parsers and registers a callback pointing to the
subcommand.
Note that the callback is a closure: that way we can immediately regain
the
control of the code flow. Also note that this function can be used
passing an
arbitrary parser. Usually a subparser is passed, but providing a normal
*ArgumentParser* this function could be reused to prepare a parser
executing
separate subcommands (e.g. `lp-init-host` or `lp-init-repo`).
- *run*: taking care of:
- namespace initialization and validation
- interactive arguments handling
- executing a dry run, if requested
- restarting the same sub command as root, if required
- actual sub command execution
Removed the subcommand dynamic dispatcher: RIP.
This is how we usually define steps::
steps = [(step1, 'arg1', 'arg2'), (step2...)]
In this branch:
steps = [(step1, ['arg1', 'arg2']), (step2, []...)]
So the second item of each sequence is now a sequence of arg names, and a third
optional element is also allowed: a filter function that takes a namespace and
must
return True if the step can be executed, False otherwise. As a side note, thanks
to this change we can now be more precise when collecting steps' descriptions.
Updated all the subcommands accordingly.
s/needs_root/root_required: needs_root is now a callable, part of the
subcommands protocol.
Removed the `lpsetup.tests.examples` module: several steps and subcommands were
defined there.
Now we have two factory test mixins in `lpsetup.tests.utils` that are used to
make
steps or subcommands when needed in tests.
Added a new context manager (*call_replaced_by*) that temporarily mocks
*subprocess.call*.
This is used to test the "restart as root" functions.
Removed the *root_not_needed* context manager: it was only used in dry run
smoke tests:
now the dry execution does not want to restart as root, because it is handled
before
`sudo` is called.
Removed no longer used *SubCommandTestMixin*, and fixed
*StepsBasedSubCommandTestMixin*:
the latter can be safely removed once we get rid of the copy/paste smoke tests.
Moved *get_step_description* from `lpsetup.utils` to `lpsetup.argparser`.
Bumped version up a little.
For more details, see:
https://code.launchpad.net/~frankban/lpsetup/out-of-control/+merge/118982
--
https://code.launchpad.net/~frankban/lpsetup/out-of-control/+merge/118982
Your team Yellow Squad is requested to review the proposed merge of
lp:~frankban/lpsetup/out-of-control into lp:lpsetup.
--
Mailing list: https://launchpad.net/~yellow
Post to : [email protected]
Unsubscribe : https://launchpad.net/~yellow
More help : https://help.launchpad.net/ListHelp
