yellow team mailing list archive
-
yellow team
-
Mailing list archive
-
Message #00958
[Merge] lp:~frankban/lpsetup/bug-1018017-renaming-things into lp:lpsetup
Francesco Banconi has proposed merging lp:~frankban/lpsetup/bug-1018017-renaming-things into lp:lpsetup.
Requested reviews:
Yellow Squad (yellow)
Related bugs:
Bug #1018017 in lpsetup: "lpsetup: Rename validators to handlers in `BaseSubCommand` and all subclasses."
https://bugs.launchpad.net/lpsetup/+bug/1018017
For more details, see:
https://code.launchpad.net/~frankban/lpsetup/bug-1018017-renaming-things/+merge/112314
== Changes ==
The validators are now called handlers.
This fix involves several changes to how things are named in the code:
- the handler that can be passed to *register_subcommand()* is now called "callback"
- the default SubCommand callback was called "handle": renamed to "run" to avoid confusion
- s/SubCommand.get_validators()/SubCommand.get_handlers()
- s/SubCommand.validate()/SubCommand.prepare_namespace()
- fixed argparse doctests: they are no longer used and we have to maintain them; should we remove them?
- changed the way handlers are imported in sub commands to avoid name clashes, e.g. `handlers = (handlers.handle_user,)`
Also updated README.rst.
--
https://code.launchpad.net/~frankban/lpsetup/bug-1018017-renaming-things/+merge/112314
Your team Yellow Squad is requested to review the proposed merge of lp:~frankban/lpsetup/bug-1018017-renaming-things into lp:lpsetup.
=== modified file 'README.rst'
--- README.rst 2012-06-25 21:17:38 +0000
+++ README.rst 2012-06-27 10:29:17 +0000
@@ -38,20 +38,22 @@
Testing the application
~~~~~~~~~~~~~~~~~~~~~~~
-To run *lpsetup* tests install nose (`apt-get install python-nose`)
-and run `nosetests` from this directory.
+To run *lpsetup* tests install nose and run `nosetests` from this directory::
+
+ apt-get install python-nose
+ nosetests
Linting
~~~~~~~
-Install pocketlint:
+Install pocketlint::
sudo apt-add-repository ppa:sinzui/ppa
sudo apt-get update
sudo apt-get install python-pocket-lint
-Run pocketlint:
+Run pocketlint::
pocketlint lpsetup/**/*.py
=== modified file 'lpsetup/argparser.py'
--- lpsetup/argparser.py 2012-06-26 18:18:23 +0000
+++ lpsetup/argparser.py 2012-06-27 10:29:17 +0000
@@ -39,7 +39,7 @@
action = super(ArgumentParser, self).add_argument(*args, **kwargs)
self.actions.append(action)
- def register_subcommand(self, name, subcommand_class, handler=None):
+ def register_subcommand(self, name, subcommand_class, callback=None):
"""Add a subcommand to this parser.
A sub command is registered giving the sub command `name` and class::
@@ -47,7 +47,7 @@
>>> parser = ArgumentParser()
>>> class SubCommand(BaseSubCommand):
- ... def handle(self, namespace):
+ ... def run(self, namespace):
... return self.name
>>> parser = ArgumentParser()
@@ -60,24 +60,24 @@
>>> namespace.main(namespace)
'foo'
- A `handler` callable can also be provided to handle the subcommand
+ A `callback` callable can also be provided to handle the subcommand
execution::
- >>> handler = lambda namespace: 'custom handler'
+ >>> callback = lambda namespace: 'custom callback'
>>> parser = ArgumentParser()
>>> _ = parser.register_subcommand(
- ... 'bar', SubCommand, handler=handler)
+ ... 'bar', SubCommand, callback=callback)
>>> namespace = parser.parse_args(['bar'])
>>> namespace.main(namespace)
- 'custom handler'
+ 'custom callback'
"""
if self.subparsers is None:
self.subparsers = self.add_subparsers(
title='subcommands',
help='Each subcommand accepts --h or --help to describe it.')
- subcommand = subcommand_class(name, handler=handler)
+ subcommand = subcommand_class(name, callback=callback)
parser = self.subparsers.add_parser(
subcommand.name, help=subcommand.help)
subcommand.add_arguments(parser)
@@ -115,8 +115,8 @@
args.append(value)
return args
- def _handle_help(self, namespace):
- """Help sub command handler."""
+ def _run_help(self, namespace):
+ """Help sub command callback."""
command = namespace.command
help = self.prefix_chars + 'h'
args = [help] if command is None else [command, help]
@@ -131,7 +131,7 @@
parser = self.subparsers.add_parser(
name, help='More help on a command.')
parser.add_argument('command', nargs='?', choices=choices)
- parser.set_defaults(main=self._handle_help)
+ parser.set_defaults(main=self._run_help)
def parse_args(self, *args, **kwargs):
"""Override to add an help sub command.
@@ -164,23 +164,26 @@
script as plugins, providing arguments, validating them, restarting
as root if needed.
- Override `add_arguments()` to add arguments, `validators` to add
- namespace validators, and `handle()` to manage sub command execution::
+ Override `add_arguments()` to add arguments, `handlers` to add
+ namespace handlers, and `run()` to manage sub command execution::
- >>> def validator(namespace):
+ >>> def handler(namespace):
... namespace.bar = True
>>> class SubCommand(BaseSubCommand):
... help = 'Sub command example.'
- ... validators = (validator,)
+ ... handlers = (handler,)
...
... def add_arguments(self, parser):
... super(SubCommand, self).add_arguments(parser)
... parser.add_argument('--foo')
...
- ... def handle(self, namespace):
+ ... def run(self, namespace):
... return namespace
+ Namespace handlers can be used to initialize and/or validate the
+ arguments namespace.
+
Register the sub command using `ArgumentParser.register_subcommand`::
>>> parser = ArgumentParser()
@@ -191,7 +194,7 @@
>>> sub_command.name
'spam'
- The sub command handler can be called using `namespace.main()`::
+ The sub command can be executed using `namespace.main()`::
>>> namespace = parser.parse_args('spam --foo eggs'.split())
>>> namespace = namespace.main(namespace)
@@ -202,6 +205,7 @@
The help attribute of sub command instances is used to generate
the command usage message::
+
>>> help = parser.format_help()
>>> 'spam' in help
True
@@ -211,11 +215,11 @@
help = ''
needs_root = False
- validators = ()
+ handlers = ()
- def __init__(self, name, handler=None):
+ def __init__(self, name, callback=None):
self.name = name
- self.handler = handler or self.handle
+ self.callback = callback or self.run
def __repr__(self):
return '<{klass}: {name}>'.format(
@@ -226,50 +230,51 @@
euid = os.geteuid()
namespace.euid, namespace.run_as_root = euid, not euid
- def get_needs_root(self, namespace):
- """Return True if root is needed to run this subcommand.
-
- Subclasses can override this to dynamically change this value
- based on the given `namespace`.
- """
- return self.needs_root
-
- def get_validators(self, namespace):
- """Return an iterable of namespace validators for this sub command.
-
- Subclasses can override this to dynamically change validators
- based on the given `namespace`.
- """
- return self.validators
-
- def validate(self, parser, namespace):
- """Validate the current namespace.
-
- The method `self.get_validators` can contain an iterable of objects
+ def get_handlers(self, namespace):
+ """Return an iterable of namespace handlers for this sub command.
+
+ Subclasses can override this to dynamically change handlers
+ based on the given `namespace`.
+ """
+ return self.handlers
+
+ def prepare_namespace(self, parser, namespace):
+ """Prepare the current namespace running namespace handlers.
+
+ The method `self.get_handlers` can return an iterable of objects
that are called once the arguments namespace is fully populated.
This allows cleaning and validating arguments that depend on
each other, or on the current environment.
- Each validator is a callable object, takes the current namespace
+ Each handler is a callable object, takes the current namespace
and can raise ValidationError if the arguments are not valid::
>>> import sys
>>> stderr, sys.stderr = sys.stderr, sys.stdout
- >>> def validator(namespace):
+ >>> def handler(namespace):
... raise ValidationError('nothing is going on')
>>> sub_command = BaseSubCommand('foo')
- >>> sub_command.validators = [validator]
- >>> sub_command.validate(ArgumentParser(), argparse.Namespace())
+ >>> sub_command.handlers = [handler]
+ >>> sub_command.prepare_namespace(
+ ... ArgumentParser(), argparse.Namespace())
Traceback (most recent call last):
SystemExit: 2
>>> sys.stderr = stderr
"""
- for validator in self.get_validators(namespace):
+ for handler in self.get_handlers(namespace):
try:
- validator(namespace)
+ handler(namespace)
except ValidationError as err:
parser.error(err)
+ def get_needs_root(self, namespace):
+ """Return True if root is needed to run this subcommand.
+
+ Subclasses can override this to dynamically change this value
+ based on the given `namespace`.
+ """
+ return self.needs_root
+
def restart_as_root(self, parser, namespace):
"""Restart this script using *sudo*.
@@ -285,24 +290,24 @@
- current argparse subparser retrieval
- namespace initialization
- - namespace validation
+ - namespace handling/validation
- script restart as root (if this sub command needs to be run as root)
- If everything is ok the sub command handler is called passing
- the validated namespace.
+ If everything is ok the sub command callback is called passing
+ the initialized/validated namespace.
"""
parser = namespace.get_parser()
self.init_namespace(namespace)
- self.validate(parser, namespace)
+ self.prepare_namespace(parser, namespace)
if self.get_needs_root(namespace) and not namespace.run_as_root:
return self.restart_as_root(parser, namespace)
- return self.handler(namespace)
+ return self.callback(namespace)
- def handle(self, namespace):
- """Default sub command handler.
+ def run(self, namespace):
+ """Default sub command callback.
Subclasses must either implement this method or provide another
- callable handler during sub command registration.
+ callback during sub command registration.
"""
raise NotImplementedError
@@ -337,7 +342,7 @@
... parser.add_argument('--foo')
... parser.add_argument('--bar')
- This class implements an handler method that executes steps in the
+ This class implements a `run` method that executes steps in the
order they are provided::
>>> parser = ArgumentParser()
@@ -368,8 +373,7 @@
>>> trace
['step2 received eggs and None']
- The steps execution is stopped if a step raises `CalledProcessError`.
- In that case, the error is returned by the handler.
+ The steps execution is stopped if a step raises an exception.
>>> trace = []
@@ -386,9 +390,9 @@
>>> parser = ArgumentParser()
>>> _ = parser.register_subcommand('sub', SubCommandWithErrors)
>>> namespace = parser.parse_args('sub --foo eggs'.split())
- >>> error = namespace.main(namespace)
- >>> str(error)
- "Command 'command' returned non-zero exit status 1"
+ >>> namespace.main(namespace)
+ Traceback (most recent call last):
+ CalledProcessError: Command 'command' returned non-zero exit status 1
The step `step2` is not executed::
@@ -483,7 +487,7 @@
"""Default callable used to run a `step`, using given `args`."""
return step(*args)
- def handle(self, namespace):
+ def run(self, namespace):
default_step_runner = self._call_step
for step_name, step, args in self.get_steps(namespace):
# Run the step using a dynamic dispatcher.
=== modified file 'lpsetup/handlers.py'
--- lpsetup/handlers.py 2012-05-22 09:58:51 +0000
+++ lpsetup/handlers.py 2012-06-27 10:29:17 +0000
@@ -2,7 +2,7 @@
# Copyright 2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-"""Sub command arguments validation."""
+"""Sub command arguments handling and validation."""
__metaclass__ = type
__all__ = [
@@ -31,7 +31,7 @@
def handle_user(namespace):
"""Handle user argument.
- This validator populates namespace with `home_dir` name::
+ This handler populates namespace with `home_dir` name::
>>> import getpass
>>> username = getpass.getuser()
=== modified file 'lpsetup/subcommands/branch.py'
--- lpsetup/subcommands/branch.py 2012-05-15 09:04:57 +0000
+++ lpsetup/subcommands/branch.py 2012-06-27 10:29:17 +0000
@@ -17,9 +17,11 @@
su,
)
-from lpsetup import (
- argparser,
- handlers,
+from lpsetup import argparser
+from lpsetup.handlers import (
+ handle_branch_creation,
+ handle_directories,
+ handle_user,
)
from lpsetup.settings import (
CHECKOUT_DIR,
@@ -57,11 +59,7 @@
'user', 'dependencies_dir', 'branch', 'make_schema'),
)
help = __doc__
- validators = (
- handlers.handle_user,
- handlers.handle_directories,
- handlers.handle_branch_creation,
- )
+ handlers = (handle_user, handle_directories, handle_branch_creation)
def add_arguments(self, parser):
super(SubCommand, self).add_arguments(parser)
=== modified file 'lpsetup/subcommands/get.py'
--- lpsetup/subcommands/get.py 2012-06-26 10:17:27 +0000
+++ lpsetup/subcommands/get.py 2012-06-27 10:29:17 +0000
@@ -21,9 +21,13 @@
su,
)
-from lpsetup import (
- argparser,
- handlers,
+from lpsetup import argparser
+from lpsetup.handlers import (
+ handle_directories,
+ handle_lpuser,
+ handle_ssh_keys,
+ handle_user,
+ handle_userdata,
)
from lpsetup.settings import (
CHECKOUT_DIR,
@@ -158,12 +162,12 @@
)
help = __doc__
- validators = (
- handlers.handle_user,
- handlers.handle_lpuser,
- handlers.handle_userdata,
- handlers.handle_ssh_keys,
- handlers.handle_directories,
+ handlers = (
+ handle_user,
+ handle_lpuser,
+ handle_userdata,
+ handle_ssh_keys,
+ handle_directories,
)
def get_needs_root(self, namespace):
=== modified file 'lpsetup/subcommands/inithost.py'
--- lpsetup/subcommands/inithost.py 2012-06-22 20:50:41 +0000
+++ lpsetup/subcommands/inithost.py 2012-06-27 10:29:17 +0000
@@ -24,9 +24,12 @@
user_exists,
)
-from lpsetup import (
- argparser,
- handlers,
+from lpsetup import argparser
+from lpsetup.handlers import (
+ handle_lpuser,
+ handle_ssh_keys,
+ handle_user,
+ handle_userdata,
)
from lpsetup.settings import (
APT_REPOSITORIES,
@@ -110,11 +113,11 @@
help = __doc__
needs_root = True
- validators = (
- handlers.handle_user,
- handlers.handle_lpuser,
- handlers.handle_userdata,
- handlers.handle_ssh_keys,
+ handlers = (
+ handle_user,
+ handle_lpuser,
+ handle_userdata,
+ handle_ssh_keys,
)
def add_arguments(self, parser):
=== modified file 'lpsetup/subcommands/install.py'
--- lpsetup/subcommands/install.py 2012-06-26 10:25:46 +0000
+++ lpsetup/subcommands/install.py 2012-06-27 10:29:17 +0000
@@ -22,7 +22,7 @@
su,
)
-from lpsetup import handlers
+from lpsetup.handlers import handle_directories
from lpsetup.subcommands import (
inithost,
get,
@@ -111,9 +111,9 @@
'user', 'dependencies_dir', 'directory', 'valid_ssh_keys'),
)
- def get_validators(self, namespace):
- validators = super(SubCommand, self).get_validators(namespace)
- return validators + (handlers.handle_directories,)
+ def get_handlers(self, namespace):
+ handlers = super(SubCommand, self).get_handlers(namespace)
+ return handlers + (handle_directories,)
def add_arguments(self, parser):
super(SubCommand, self).add_arguments(parser)
=== modified file 'lpsetup/subcommands/lxcinstall.py'
--- lpsetup/subcommands/lxcinstall.py 2012-06-26 10:25:46 +0000
+++ lpsetup/subcommands/lxcinstall.py 2012-06-27 10:29:17 +0000
@@ -25,10 +25,8 @@
mkdirs,
)
-from lpsetup import (
- exceptions,
- handlers,
- )
+from lpsetup import exceptions
+from lpsetup.handlers import handle_testing
from lpsetup.settings import (
BASE_PACKAGES,
LPSETUP_PPA,
@@ -227,9 +225,9 @@
)
help = __doc__
- def get_validators(self, namespace):
- validators = super(SubCommand, self).get_validators(namespace)
- return validators + (handlers.handle_testing,)
+ def get_handlers(self, namespace):
+ handlers = super(SubCommand, self).get_handlers(namespace)
+ return handlers + (handle_testing,)
def call_create_scripts(self, namespace, step, args):
"""Run the `create_scripts` step only if the related flag is set."""
=== modified file 'lpsetup/subcommands/update.py'
--- lpsetup/subcommands/update.py 2012-06-25 17:55:10 +0000
+++ lpsetup/subcommands/update.py 2012-06-27 10:29:17 +0000
@@ -9,7 +9,11 @@
'SubCommand',
]
-from lpsetup import handlers
+from lpsetup.handlers import (
+ handle_directories,
+ handle_ssh_keys,
+ handle_user,
+ )
from lpsetup.subcommands import get
@@ -22,11 +26,7 @@
get.SubCommand.link_sourcecode_in_branches_step,
)
help = __doc__
- validators = (
- handlers.handle_user,
- handlers.handle_directories,
- handlers.handle_ssh_keys,
- )
+ handlers = (handle_user, handle_directories, handle_ssh_keys)
def add_arguments(self, parser):
super(get.SubCommand, self).add_arguments(parser)
=== modified file 'lpsetup/subcommands/version.py'
--- lpsetup/subcommands/version.py 2012-05-22 09:58:51 +0000
+++ lpsetup/subcommands/version.py 2012-06-27 10:29:17 +0000
@@ -23,6 +23,6 @@
help = __doc__
- def handle(self, namespace):
+ def run(self, namespace):
app_name = os.path.basename(sys.argv[0])
print '{0} v{1}'.format(app_name, get_version())
=== modified file 'lpsetup/tests/examples.py'
--- lpsetup/tests/examples.py 2012-03-30 10:27:46 +0000
+++ lpsetup/tests/examples.py 2012-06-27 10:29:17 +0000
@@ -41,7 +41,7 @@
super(SubCommand, self).add_arguments(parser)
parser.add_argument('--foo')
- def handle(self, namespace):
+ def run(self, namespace):
return namespace
=== modified file 'lpsetup/tests/test_argparser.py'
--- lpsetup/tests/test_argparser.py 2012-06-26 18:33:50 +0000
+++ lpsetup/tests/test_argparser.py 2012-06-27 10:29:17 +0000
@@ -25,7 +25,7 @@
def get_sub_command(self):
return type(
'SubCommand', (argparser.BaseSubCommand,),
- {'handle': lambda self, namespace: self.name})
+ {'run': lambda self, namespace: self.name})
def test_add_argument(self):
# Ensure actions are stored in a "public" instance attribute.
@@ -45,11 +45,11 @@
def test_register_subcommand_providing_handler(self):
# Ensure a `handler` callable can also be provided to handle the
# subcommand execution.
- handler = lambda namespace: 'custom handler'
+ callback = lambda namespace: 'custom handler'
self.parser.register_subcommand(
- 'bar', self.get_sub_command(), handler=handler)
+ 'bar', self.get_sub_command(), callback=callback)
namespace = self.parser.parse_args(['bar'])
- self.assertEqual(handler(namespace), namespace.main(namespace))
+ self.assertEqual(callback(namespace), namespace.main(namespace))
def test_get_args_from_namespace(self):
# It is possible to recreate the argument list taking values from
@@ -82,13 +82,13 @@
class BaseSubCommandTest(SubCommandTestMixin, unittest.TestCase):
- def set_validators(self, *args):
- self.sub_command.validators = args
+ def set_handlers(self, *args):
+ self.sub_command.handlers = args
- def _successful_validator(self, namespace):
+ def _successful_handler(self, namespace):
namespace.bar = True
- def _failing_validator(self, namespace):
+ def _failing_handler(self, namespace):
raise ValidationError('nothing is going on')
def test_name(self):
@@ -101,14 +101,14 @@
self.assertEqual('eggs', namespace.foo)
def test_successful_validation(self):
- # Ensure attached validators are called by the default handler.
- self.set_validators(self._successful_validator)
+ # Ensure attached handlers are called by the default callback.
+ self.set_handlers(self._successful_handler)
namespace = self.parse_and_call_main()
self.assertTrue(namespace.bar)
def test_failing_validation(self):
- # Ensure `ValidationError` stops the handler execution.
- self.set_validators(self._failing_validator)
+ # Ensure `ValidationError` stops the command execution.
+ self.set_handlers(self._failing_handler)
with self.assertRaises(SystemExit) as cm:
with capture_error() as error:
self.parse_and_call_main()
=== modified file 'lpsetup/tests/utils.py'
--- lpsetup/tests/utils.py 2012-06-26 15:02:25 +0000
+++ lpsetup/tests/utils.py 2012-06-27 10:29:17 +0000
@@ -64,7 +64,7 @@
namespace = self.parser.parse_args((self.sub_command_name,) + args)
sub_command = self.sub_command
sub_command.init_namespace(namespace)
- sub_command.validate(self.parser, namespace)
+ sub_command.prepare_namespace(self.parser, namespace)
return namespace
def parse_and_call_main(self, *args):
@@ -98,7 +98,7 @@
def test_handlers(self):
# Ensure this sub command uses the expected handlers.
- handlers = self.sub_command.get_validators(self.namespace)
+ handlers = self.sub_command.get_handlers(self.namespace)
self.assertSequenceEqual(self.expected_handlers, handlers)
def test_steps(self):
