From 5c0b8c2b0229992671e076e74c1256a880381d62 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Tue, 16 Sep 2014 15:50:04 -0700 Subject: testsuite: Added unit tests for new option parsing --- src/lib/Bcfg2/Options/Actions.py | 12 +- src/lib/Bcfg2/Options/OptionGroups.py | 61 ++++---- src/lib/Bcfg2/Options/Options.py | 128 +++++++++++++---- src/lib/Bcfg2/Options/Parser.py | 94 +++++++------ src/lib/Bcfg2/Options/Subcommands.py | 153 +++++++++++---------- src/lib/Bcfg2/Options/Types.py | 10 +- src/lib/Bcfg2/Reporting/Reports.py | 3 +- src/lib/Bcfg2/Server/Admin.py | 14 +- src/lib/Bcfg2/Server/Info.py | 25 +--- src/lib/Bcfg2/Server/Plugins/Packages/YumHelper.py | 4 +- 10 files changed, 305 insertions(+), 199 deletions(-) (limited to 'src/lib') diff --git a/src/lib/Bcfg2/Options/Actions.py b/src/lib/Bcfg2/Options/Actions.py index 8b941f2bb..2916cc194 100644 --- a/src/lib/Bcfg2/Options/Actions.py +++ b/src/lib/Bcfg2/Options/Actions.py @@ -2,7 +2,8 @@ import sys import argparse -from Bcfg2.Options.Parser import get_parser +from Bcfg2.Options.Parser import get_parser, OptionParserException +from Bcfg2.Options.Options import _debug __all__ = ["ConfigFileAction", "ComponentAction", "PluginsAction"] @@ -101,7 +102,7 @@ class ComponentAction(FinalizableAction): fail_silently = False def __init__(self, *args, **kwargs): - if self.mapping: + if self.mapping and not self.islist: if 'choices' not in kwargs: kwargs['choices'] = self.mapping.keys() FinalizableAction.__init__(self, *args, **kwargs) @@ -143,7 +144,7 @@ class ComponentAction(FinalizableAction): if cls: get_parser().add_component(cls) elif not self.fail_silently: - print("Could not load component %s" % name) + raise OptionParserException("Could not load component %s" % name) return cls def __call__(self, parser, namespace, values, option_string=None): @@ -168,7 +169,10 @@ class ConfigFileAction(FinalizableAction): ``bcfg2-lint.conf``). """ def __call__(self, parser, namespace, values, option_string=None): - parser.add_config_file(self.dest, values, reparse=False) + if values: + parser.add_config_file(self.dest, values, reparse=False) + else: + _debug("No config file passed for %s" % self) FinalizableAction.__call__(self, parser, namespace, values, option_string=option_string) diff --git a/src/lib/Bcfg2/Options/OptionGroups.py b/src/lib/Bcfg2/Options/OptionGroups.py index 465358fab..426ee5dc0 100644 --- a/src/lib/Bcfg2/Options/OptionGroups.py +++ b/src/lib/Bcfg2/Options/OptionGroups.py @@ -10,7 +10,7 @@ __all__ = ["OptionGroup", "ExclusiveOptionGroup", "Subparser", "WildcardSectionGroup"] -class OptionContainer(list): +class _OptionContainer(list): """ Parent class of all option groups """ def list_options(self): @@ -29,7 +29,7 @@ class OptionContainer(list): opt.add_to_parser(parser) -class OptionGroup(OptionContainer): +class OptionGroup(_OptionContainer): """ Generic option group that is used only to organize options. This uses :meth:`argparse.ArgumentParser.add_argument_group` behind the scenes. """ @@ -43,16 +43,16 @@ class OptionGroup(OptionContainer): :param description: A longer description of the option group :param description: string """ - OptionContainer.__init__(self, items) + _OptionContainer.__init__(self, items) self.title = kwargs.pop('title') self.description = kwargs.pop('description', None) def add_to_parser(self, parser): group = parser.add_argument_group(self.title, self.description) - OptionContainer.add_to_parser(self, group) + _OptionContainer.add_to_parser(self, group) -class ExclusiveOptionGroup(OptionContainer): +class ExclusiveOptionGroup(_OptionContainer): """ Option group that ensures that only one argument in the group is present. This uses :meth:`argparse.ArgumentParser.add_mutually_exclusive_group` @@ -66,15 +66,15 @@ class ExclusiveOptionGroup(OptionContainer): specified. :type required: boolean """ - OptionContainer.__init__(self, items) + _OptionContainer.__init__(self, items) self.required = kwargs.pop('required', False) def add_to_parser(self, parser): group = parser.add_mutually_exclusive_group(required=self.required) - OptionContainer.add_to_parser(self, group) + _OptionContainer.add_to_parser(self, group) -class Subparser(OptionContainer): +class Subparser(_OptionContainer): """ Option group that adds options in it to a subparser. This uses a lot of functionality tied to `argparse Sub-commands `_. @@ -99,7 +99,7 @@ class Subparser(OptionContainer): """ self.name = kwargs.pop('name') self.help = kwargs.pop('help', None) - OptionContainer.__init__(self, items) + _OptionContainer.__init__(self, items) def __repr__(self): return "%s %s(%s)" % (self.__class__.__name__, @@ -111,11 +111,11 @@ class Subparser(OptionContainer): self._subparsers[parser] = parser.add_subparsers(dest='subcommand') subparser = self._subparsers[parser].add_parser(self.name, help=self.help) - OptionContainer.add_to_parser(self, subparser) + _OptionContainer.add_to_parser(self, subparser) -class WildcardSectionGroup(OptionContainer, Option): - """ WildcardSectionGroups contain options that may exist in +class WildcardSectionGroup(_OptionContainer, Option): + """WildcardSectionGroups contain options that may exist in several different sections of the config that match a glob. It works by creating options on the fly to match the sections described in the glob. For example, consider: @@ -134,7 +134,7 @@ class WildcardSectionGroup(OptionContainer, Option): .. code-block:: python >>> Bcfg2.Options.setup - Namespace(myplugin_bar_description='Bar description', myplugin_bar_number=2, myplugin_foo_description='Foo description', myplugin_foo_number=1, myplugin_sections=['myplugin:foo', 'myplugin:bar']) + Namespace(myplugin_bar_description='Bar description', myplugin_myplugin_bar_number=2, myplugin_myplugin_foo_description='Foo description', myplugin_myplugin_foo_number=1, myplugin_sections=['myplugin:foo', 'myplugin:bar']) All options must have the same section glob. @@ -146,10 +146,10 @@ class WildcardSectionGroup(OptionContainer, Option): ```` is the original `dest `_ of the option. ``
`` is the section that it's found in. - ```` is automatically generated from the section glob by - replacing all consecutive characters disallowed in Python variable - names into underscores. (This can be overridden with the - constructor.) + ```` is automatically generated from the section glob. + (This can be overridden with the constructor.) Both ``
`` + and ```` have had all consecutive characters disallowed in + Python variable names replaced with underscores. This group stores an additional option, the sections themselves, in an option given by ``sections``. @@ -171,17 +171,17 @@ class WildcardSectionGroup(OptionContainer, Option): that match the glob. :param dest: string """ - OptionContainer.__init__(self, []) + _OptionContainer.__init__(self, []) self._section_glob = items[0].cf[0] # get a default destination self._prefix = kwargs.get("prefix", self._dest_re.sub('_', self._section_glob)) Option.__init__(self, dest=kwargs.get('dest', self._prefix + "sections")) - self._options = items + self.option_templates = items def list_options(self): - return [self] + OptionContainer.list_options(self) + return [self] + _OptionContainer.list_options(self) def from_config(self, cfp): sections = [] @@ -189,10 +189,12 @@ class WildcardSectionGroup(OptionContainer, Option): if fnmatch.fnmatch(section, self._section_glob): sections.append(section) newopts = [] - for opt_tmpl in self._options: + for opt_tmpl in self.option_templates: option = copy.deepcopy(opt_tmpl) option.cf = (section, option.cf[1]) - option.dest = self._prefix + section + "_" + option.dest + option.dest = "%s%s_%s" % (self._prefix, + self._dest_re.sub('_', section), + option.dest) newopts.append(option) self.extend(newopts) for parser in self.parsers: @@ -201,4 +203,17 @@ class WildcardSectionGroup(OptionContainer, Option): def add_to_parser(self, parser): Option.add_to_parser(self, parser) - OptionContainer.add_to_parser(self, parser) + _OptionContainer.add_to_parser(self, parser) + + def __eq__(self, other): + return (_OptionContainer.__eq__(self, other) and + self.option_templates == getattr(other, "option_templates", + None)) + + def __repr__(self): + if len(self) == 0: + return "%s(%s)" % (self.__class__.__name__, + ", ".join(".".join(o.cf) + for o in self.option_templates)) + else: + return _OptionContainer.__repr__(self) diff --git a/src/lib/Bcfg2/Options/Options.py b/src/lib/Bcfg2/Options/Options.py index 3874f810d..4efd76929 100644 --- a/src/lib/Bcfg2/Options/Options.py +++ b/src/lib/Bcfg2/Options/Options.py @@ -1,11 +1,15 @@ -""" The base :class:`Bcfg2.Options.Option` object represents an -option. Unlike options in :mod:`argparse`, an Option object does not -need to be associated with an option parser; it exists on its own.""" +"""Base :class:`Bcfg2.Options.Option` object to represent an option. -import os +Unlike options in :mod:`argparse`, an Option object does not need to +be associated with an option parser; it exists on its own. +""" + +import argparse import copy import fnmatch -import argparse +import os +import sys + from Bcfg2.Options import Types from Bcfg2.Compat import ConfigParser @@ -19,8 +23,9 @@ def _debug(msg): they're options, after all -- so option parsing verbosity is enabled by changing this to True. The verbosity here is primarily of use to developers. """ - if os.environ.get('BCFG2_OPTIONS_DEBUG', '0') == '1': - print(msg) + if os.environ.get('BCFG2_OPTIONS_DEBUG', '0').lower() in ["true", "yes", + "on", "1"]: + sys.stderr.write("%s\n" % msg) #: A dict that records a mapping of argparse action name (e.g., @@ -133,10 +138,11 @@ class Option(object): self._dest = None if 'dest' in self._kwargs: self._dest = self._kwargs.pop('dest') - elif self.cf is not None: - self._dest = self.cf[1] elif self.env is not None: self._dest = self.env + elif self.cf is not None: + self._dest = self.cf[1] + self._dest = self._dest.lower().replace("-", "_") kwargs = copy.copy(self._kwargs) kwargs.pop("action", None) self.actions[None] = action_cls(self._dest, self._dest, **kwargs) @@ -175,6 +181,17 @@ class Option(object): _debug("Finalizing %s" % self) action.finalize(parser, namespace) + @property + def _type_func(self): + """get a function for converting a value to the option type. + + this always returns a callable, even when ``type`` is None. + """ + if self.type: + return self.type + else: + return lambda x: x + def from_config(self, cfp): """ Get the value of this option from the given :class:`ConfigParser.ConfigParser`. If it is not found in the @@ -201,21 +218,33 @@ class Option(object): self.cf[1]) if o not in exclude]) else: - rv = dict() + rv = {} else: - if self.type: - rtype = self.type - else: - rtype = lambda x: x try: - rv = rtype(cfp.getboolean(*self.cf)) - except ValueError: - rv = rtype(cfp.get(*self.cf)) + rv = self._type_func(self.get_config_value(cfp)) except (ConfigParser.NoSectionError, ConfigParser.NoOptionError): rv = None _debug("Setting %s from config file(s): %s" % (self, rv)) return rv + def get_config_value(self, cfp): + """fetch a value from the config file. + + This is passed the config parser. Its result is passed to the + type function for this option. It can be overridden to, e.g., + handle boolean options. + """ + return cfp.get(*self.cf) + + def get_environ_value(self, value): + """fetch a value from the environment. + + This is passed the raw value from the environment variable, + and its result is passed to the type function for this + option. It can be overridden to, e.g., handle boolean options. + """ + return value + def default_from_config(self, cfp): """ Set the default value of this option from the config file or from the environment. @@ -224,7 +253,8 @@ class Option(object): :type cfp: ConfigParser.ConfigParser """ if self.env and self.env in os.environ: - self.default = os.environ[self.env] + self.default = self._type_func( + self.get_environ_value(os.environ[self.env])) _debug("Setting the default of %s from environment: %s" % (self, self.default)) else: @@ -257,6 +287,13 @@ class Option(object): for action in self.actions.values(): action.dest = value + def early_parsing_hook(self, early_opts): # pylint: disable=C0111 + """Hook called at the end of early option parsing. + + This can be used to save option values for macro fixup. + """ + pass + #: The namespace destination of this option (see `dest #: `_) dest = property(_get_dest, _set_dest) @@ -289,8 +326,8 @@ class PathOption(Option): :meth:`Bcfg2.Options.Types.path` to transform the argument into a canonical path. - The type of a path option can also be overridden to return an - option file-like object. For example: + The type of a path option can also be overridden to return a + file-like object. For example: .. code-block:: python @@ -301,27 +338,52 @@ class PathOption(Option): """ def __init__(self, *args, **kwargs): - kwargs.setdefault('type', Types.path) + self._original_type = kwargs.pop('type', lambda x: x) + kwargs['type'] = self._type kwargs.setdefault('metavar', '') Option.__init__(self, *args, **kwargs) + self.repository = None + + def early_parsing_hook(self, early_opts): + self.repository = early_opts.repository + + def _type(self, value): + """Type function that fixes up macros.""" + if self.repository is None: + rv = value + else: + rv = value.replace("", self.repository) + return self._original_type(Types.path(rv)) class _BooleanOptionAction(argparse.Action): - """ BooleanOptionAction sets a boolean value in the following ways: + """BooleanOptionAction sets a boolean value. + - if None is passed, store the default - if the option_string is not None, then the option was passed on the command line, thus store the opposite of the default (this is the argparse store_true and store_false behavior) - if a boolean value is passed, use that + Makes a copy of the initial default, because otherwise the default + can be changed by config file settings or environment + variables. For instance, if a boolean option that defaults to True + was set to False in the config file, specifying the option on the + CLI would then set it back to True. + Defined here instead of :mod:`Bcfg2.Options.Actions` because otherwise - there is a circular import Options -> Actions -> Parser -> Options """ + there is a circular import Options -> Actions -> Parser -> Options. + """ + + def __init__(self, *args, **kwargs): + argparse.Action.__init__(self, *args, **kwargs) + self.original = self.default def __call__(self, parser, namespace, values, option_string=None): if values is None: setattr(namespace, self.dest, self.default) elif option_string is not None: - setattr(namespace, self.dest, not self.default) + setattr(namespace, self.dest, not self.original) else: setattr(namespace, self.dest, bool(values)) @@ -340,9 +402,25 @@ class BooleanOption(Option): kwargs.setdefault('action', _BooleanOptionAction) kwargs.setdefault('nargs', 0) kwargs.setdefault('default', False) - Option.__init__(self, *args, **kwargs) + def get_environ_value(self, value): + if value.lower() in ["false", "no", "off", "0"]: + return False + elif value.lower() in ["true", "yes", "on", "1"]: + return True + else: + raise ValueError("Invalid boolean value %s" % value) + + def get_config_value(self, cfp): + """fetch a value from the config file. + + This is passed the config parser. Its result is passed to the + type function for this option. It can be overridden to, e.g., + handle boolean options. + """ + return cfp.getboolean(*self.cf) + class PositionalArgument(Option): """ Shortcut for positional arguments. """ diff --git a/src/lib/Bcfg2/Options/Parser.py b/src/lib/Bcfg2/Options/Parser.py index 677a69e4c..ec470f8d3 100644 --- a/src/lib/Bcfg2/Options/Parser.py +++ b/src/lib/Bcfg2/Options/Parser.py @@ -1,13 +1,15 @@ -""" The option parser """ +"""The option parser.""" +import argparse import os import sys -import argparse + from Bcfg2.version import __version__ from Bcfg2.Compat import ConfigParser from Bcfg2.Options import Option, PathOption, BooleanOption, _debug -__all__ = ["setup", "OptionParserException", "Parser", "get_parser"] +__all__ = ["setup", "OptionParserException", "Parser", "get_parser", + "new_parser"] #: The repository option. This is specified here (and imported into @@ -108,13 +110,28 @@ class Parser(argparse.ArgumentParser): for component in components: self.add_component(component) + def _check_duplicate_cf(self, option): + """Check for a duplicate config file option.""" + def add_options(self, options): """ Add an explicit list of options to the parser. When possible, prefer :func:`Bcfg2.Options.Parser.add_component` to add a whole component instead.""" + _debug("Adding options: %s" % options) self.parsed = False for option in options: if option not in self.option_list: + # check for duplicates + if (hasattr(option, "env") and option.env and + option.env in [o.env for o in self.option_list]): + raise OptionParserException( + "Duplicate environment variable option: %s" % + option.env) + if (hasattr(option, "cf") and option.cf and + option.cf in [o.cf for o in self.option_list]): + raise OptionParserException( + "Duplicate config file option: %s" % (option.cf,)) + self.option_list.extend(option.list_options()) option.add_to_parser(self) @@ -169,8 +186,8 @@ class Parser(argparse.ArgumentParser): _debug("Resetting namespace") for attr in dir(self.namespace): if (not attr.startswith("_") and - attr not in ['uri', 'version', 'name'] and - attr not in self._config_files): + attr not in ['uri', 'version', 'name'] and + attr not in self._config_files): _debug("Deleting %s" % attr) delattr(self.namespace, attr) @@ -210,7 +227,7 @@ class Parser(argparse.ArgumentParser): """ _debug("Parsing options") if argv is None: - argv = sys.argv[1:] + argv = sys.argv[1:] # pragma: nocover if self.parsed and self.argv == argv: _debug("Returning already parsed namespace") return self.namespace @@ -231,8 +248,8 @@ class Parser(argparse.ArgumentParser): # phase 2: re-parse command line for early options; currently, # that's database options - _debug("Option parsing phase 2: Parse early options") if not self._early: + _debug("Option parsing phase 2: Parse early options") early_opts = argparse.Namespace() early_parser = Parser(add_help=False, namespace=early_opts, early=True) @@ -250,6 +267,10 @@ class Parser(argparse.ArgumentParser): if hasattr(component, "component_parsed_hook"): _debug("Calling component_parsed_hook on %s" % component) getattr(component, "component_parsed_hook")(early_opts) + for option in self.option_list: + option.early_parsing_hook(early_opts) + else: + _debug("Skipping parsing phase 2 in early mode") # phase 3: re-parse command line, loading additional # components, until all components have been loaded. On each @@ -275,29 +296,24 @@ class Parser(argparse.ArgumentParser): # namespace and _parse_config_options will never look at them again. self._set_defaults_from_config() self._parse_config_options() + remaining = [] while not self.parsed: self.parsed = True self._set_defaults_from_config() - self.parse_known_args(args=self.argv, namespace=self.namespace) + _debug("Parsing known arguments") + try: + _, remaining = self.parse_known_args(args=self.argv, + namespace=self.namespace) + except OptionParserException: + self.error(sys.exc_info()[1]) self._parse_config_options() self._finalize() + if len(remaining) and not self._early: + self.error("Unknown options: %s" % " ".join(remaining)) - # phase 4: fix up macros - _debug("Option parsing phase 4: Fix up macros") - repo = getattr(self.namespace, "repository", repository.default) - for attr in dir(self.namespace): - value = getattr(self.namespace, attr) - if (not attr.startswith("_") and - hasattr(value, "replace") and - "" in value): - setattr(self.namespace, attr, - value.replace("", repo, 1)) - _debug("Fixing up macros in %s: %s -> %s" % - (attr, value, getattr(self.namespace, attr))) - - # phase 5: call post-parsing hooks - _debug("Option parsing phase 5: Call hooks") + # phase 4: call post-parsing hooks if not self._early: + _debug("Option parsing phase 4: Call hooks") for component in self.components: if hasattr(component, "options_parsed_hook"): _debug("Calling post-parsing hook on %s" % component) @@ -311,23 +327,23 @@ class Parser(argparse.ArgumentParser): _parser = Parser() # pylint: disable=C0103 +def new_parser(): + """Create a new :class:`Bcfg2.Options.Parser` object. + + The new object can be retrieved with + :func:`Bcfg2.Options.get_parser`. This is useful for unit + testing. + """ + global _parser + _parser = Parser() + + def get_parser(description=None, components=None, namespace=None): - """ Get an existing :class:`Bcfg2.Options.Parser` object. (One is - created at the module level when :mod:`Bcfg2.Options` is - imported.) If no arguments are given, then the existing parser is - simply fetched. - - If arguments are given, then one of two things happens: - - * If this is the first ``get_parser`` call with arguments, then - the values given are set accordingly in the parser, and it is - returned. - * If this is not the first such call, then - :class:`Bcfg2.Options.OptionParserException` is raised. - - That is, a ``get_parser`` call with options is considered to - initialize the parser that already exists, and that can only - happen once. + """Get an existing :class:`Bcfg2.Options.Parser` object. + + A Parser is created at the module level when :mod:`Bcfg2.Options` + is imported. If any arguments are given, then the existing parser + is modified before being returned. :param description: Set the parser description :type description: string diff --git a/src/lib/Bcfg2/Options/Subcommands.py b/src/lib/Bcfg2/Options/Subcommands.py index 660bd5077..8972bde00 100644 --- a/src/lib/Bcfg2/Options/Subcommands.py +++ b/src/lib/Bcfg2/Options/Subcommands.py @@ -7,12 +7,13 @@ import sys import copy import shlex import logging + from Bcfg2.Compat import StringIO -from Bcfg2.Options import PositionalArgument +from Bcfg2.Options import PositionalArgument, _debug from Bcfg2.Options.OptionGroups import Subparser from Bcfg2.Options.Parser import Parser, setup as master_setup -__all__ = ["Subcommand", "HelpCommand", "CommandRegistry", "register_commands"] +__all__ = ["Subcommand", "CommandRegistry"] class Subcommand(object): @@ -96,8 +97,8 @@ class Subcommand(object): sio = StringIO() self.parser.print_usage(file=sio) usage = self._ws_re.sub(' ', sio.getvalue()).strip()[7:] - doc = self._ws_re.sub(' ', getattr(self, "__doc__")).strip() - if doc is None: + doc = self._ws_re.sub(' ', getattr(self, "__doc__") or "").strip() + if not doc: self._usage = usage else: self._usage = "%s - %s" % (usage, doc) @@ -119,120 +120,130 @@ class Subcommand(object): command from the interactive shell. :type setup: argparse.Namespace """ - raise NotImplementedError + raise NotImplementedError # pragma: nocover def shutdown(self): - """ Perform any necessary shtudown tasks for this command This + """ Perform any necessary shutdown tasks for this command This is called to when the program exits (*not* when this command is finished executing). """ - pass + pass # pragma: nocover -class HelpCommand(Subcommand): - """ Get help on a specific subcommand. This must be subclassed to - create the actual help command by overriding - :func:`Bcfg2.Options.HelpCommand.command_registry` and giving the - command access to a :class:`Bcfg2.Options.CommandRegistry`. """ +class Help(Subcommand): + """List subcommands and usage, or get help on a specific subcommand.""" options = [PositionalArgument("command", nargs='?')] # the interactive shell has its own help interactive = False - def command_registry(self): - """ Return a :class:`Bcfg2.Options.CommandRegistry` class. - All commands registered with the class will be included in the - help message. """ - raise NotImplementedError + def __init__(self, registry): + Subcommand.__init__(self) + self._registry = registry def run(self, setup): - commands = self.command_registry() + commands = self._registry.commands if setup.command: try: commands[setup.command].parser.print_help() return 0 except KeyError: print("No such command: %s" % setup.command) + return 1 for command in sorted(commands.keys()): print(commands[command].usage()) class CommandRegistry(object): - """ A ``CommandRegistry`` is used to register subcommands and - provides a single interface to run them. It's also used by - :class:`Bcfg2.Options.HelpCommand` to produce help messages for - all available commands. """ + """A ``CommandRegistry`` is used to register subcommands and provides + a single interface to run them. It's also used by + :class:`Bcfg2.Options.Subcommands.Help` to produce help messages + for all available commands. + """ + + def __init__(self): + #: A dict of registered commands. Keys are the class names, + #: lowercased (i.e., the command names), and values are instances + #: of the command objects. + self.commands = dict() - #: A dict of registered commands. Keys are the class names, - #: lowercased (i.e., the command names), and values are instances - #: of the command objects. - commands = dict() + #: A list of options that should be added to the option parser + #: in order to handle registered subcommands. + self.subcommand_options = [] - options = [] + #: the help command + self.help = Help(self) + self.register_command(self.help) def runcommand(self): """ Run the single command named in ``Bcfg2.Options.setup.subcommand``, which is where :class:`Bcfg2.Options.Subparser` groups store the subcommand. """ + _debug("Running subcommand %s" % master_setup.subcommand) try: return self.commands[master_setup.subcommand].run(master_setup) finally: self.shutdown() def shutdown(self): - """ Perform shutdown tasks. This calls the ``shutdown`` - method of all registered subcommands. """ + """Perform shutdown tasks. + + This calls the ``shutdown`` method of the subcommand that was + run. + """ + _debug("Shutting down subcommand %s" % master_setup.subcommand) self.commands[master_setup.subcommand].shutdown() - @classmethod - def register_command(cls, cmdcls): + def register_command(self, cls_or_obj): """ Register a single command. - :param cmdcls: The command class to register - :type cmdcls: type + :param cls_or_obj: The command class or object to register + :type cls_or_obj: type or Subcommand :returns: An instance of ``cmdcls`` """ - cmd_obj = cmdcls() + if isinstance(cls_or_obj, type): + cmdcls = cls_or_obj + cmd_obj = cmdcls() + else: + cmd_obj = cls_or_obj + cmdcls = cmd_obj.__class__ name = cmdcls.__name__.lower() - cls.commands[name] = cmd_obj + self.commands[name] = cmd_obj # py2.5 can't mix *magic and non-magical keyword args, thus # the **dict(...) - cls.options.append( + self.subcommand_options.append( Subparser(*cmdcls.options, **dict(name=name, help=cmdcls.__doc__))) - if issubclass(cls, cmd.Cmd) and cmdcls.interactive: - setattr(cls, "do_%s" % name, cmd_obj) - setattr(cls, "help_%s" % name, cmd_obj.parser.print_help) + if issubclass(self.__class__, cmd.Cmd) and cmdcls.interactive: + setattr(self, "do_%s" % name, cmd_obj) + setattr(self, "help_%s" % name, cmd_obj.parser.print_help) return cmd_obj - -def register_commands(registry, candidates, parent=Subcommand): - """ Register all subcommands in ``candidates`` against the - :class:`Bcfg2.Options.CommandRegistry` subclass given in - ``registry``. A command is registered if and only if: - - * It is a subclass of the given ``parent`` (by default, - :class:`Bcfg2.Options.Subcommand`); - * It is not the parent class itself; and - * Its name does not start with an underscore. - - :param registry: The :class:`Bcfg2.Options.CommandRegistry` - subclass against which commands will be - registered. - :type registry: Bcfg2.Options.CommandRegistry - :param candidates: A list of objects that will be considered for - registration. Only objects that meet the - criteria listed above will be registered. - :type candidates: list - :param parent: Specify a parent class other than - :class:`Bcfg2.Options.Subcommand` that all - registered commands must subclass. - :type parent: type - """ - for attr in candidates: - try: - if (issubclass(attr, parent) and - attr != parent and - not attr.__name__.startswith("_")): - registry.register_command(attr) - except TypeError: - pass + def register_commands(self, candidates, parent=Subcommand): + """ Register all subcommands in ``candidates`` against the + :class:`Bcfg2.Options.CommandRegistry` subclass given in + ``registry``. A command is registered if and only if: + + * It is a subclass of the given ``parent`` (by default, + :class:`Bcfg2.Options.Subcommand`); + * It is not the parent class itself; and + * Its name does not start with an underscore. + + :param registry: The :class:`Bcfg2.Options.CommandRegistry` + subclass against which commands will be + registered. + :type registry: Bcfg2.Options.CommandRegistry + :param candidates: A list of objects that will be considered for + registration. Only objects that meet the + criteria listed above will be registered. + :type candidates: list + :param parent: Specify a parent class other than + :class:`Bcfg2.Options.Subcommand` that all + registered commands must subclass. + :type parent: type + """ + for attr in candidates: + if (isinstance(attr, type) and + issubclass(attr, parent) and + attr != parent and + not attr.__name__.startswith("_")): + self.register_command(attr) diff --git a/src/lib/Bcfg2/Options/Types.py b/src/lib/Bcfg2/Options/Types.py index d11e54fba..1c04fede3 100644 --- a/src/lib/Bcfg2/Options/Types.py +++ b/src/lib/Bcfg2/Options/Types.py @@ -38,9 +38,11 @@ def comma_dict(value): for item in items: if '=' in item: key, value = item.split(r'=', 1) - try: - result[key] = bool(value) - except ValueError: + if value in ["true", "yes", "on"]: + result[key] = True + elif value in ["false", "no", "off"]: + result[key] = False + else: try: result[key] = int(value) except ValueError: @@ -107,8 +109,6 @@ def size(value): """ Given a number of bytes in a human-readable format (e.g., '512m', '2g'), get the absolute number of bytes as an integer. """ - if value == -1: - return value mat = _bytes_re.match(value) if not mat: raise ValueError("Not a valid size", value) diff --git a/src/lib/Bcfg2/Reporting/Reports.py b/src/lib/Bcfg2/Reporting/Reports.py index 219d74584..3b9c83433 100755 --- a/src/lib/Bcfg2/Reporting/Reports.py +++ b/src/lib/Bcfg2/Reporting/Reports.py @@ -267,10 +267,11 @@ class CLI(Bcfg2.Options.CommandRegistry): def __init__(self): Bcfg2.Options.CommandRegistry.__init__(self) - Bcfg2.Options.register_commands(self.__class__, globals().values()) + self.register_commands(globals().values()) parser = Bcfg2.Options.get_parser( description="Query the Bcfg2 reporting subsystem", components=[self]) + parser.add_options(self.subcommand_options) parser.parse() def run(self): diff --git a/src/lib/Bcfg2/Server/Admin.py b/src/lib/Bcfg2/Server/Admin.py index 0807fb2b0..b6dded175 100644 --- a/src/lib/Bcfg2/Server/Admin.py +++ b/src/lib/Bcfg2/Server/Admin.py @@ -439,15 +439,6 @@ class Compare(AdminCmd): print("") -class Help(AdminCmd, Bcfg2.Options.HelpCommand): - """ Get help on a specific subcommand """ - def command_registry(self): - return CLI.commands - - def run(self, setup): - Bcfg2.Options.HelpCommand.run(self, setup) - - class Init(AdminCmd): """Interactively initialize a new repository.""" @@ -1194,13 +1185,14 @@ class Xcmd(_ProxyAdminCmd): class CLI(Bcfg2.Options.CommandRegistry): """ CLI class for bcfg2-admin """ + def __init__(self): Bcfg2.Options.CommandRegistry.__init__(self) - Bcfg2.Options.register_commands(self.__class__, globals().values(), - parent=AdminCmd) + self.register_commands(globals().values(), parent=AdminCmd) parser = Bcfg2.Options.get_parser( description="Manage a running Bcfg2 server", components=[self]) + parser.add_options(self.subcommand_options) parser.parse() def run(self): diff --git a/src/lib/Bcfg2/Server/Info.py b/src/lib/Bcfg2/Server/Info.py index a5136f01d..0baa29a9f 100644 --- a/src/lib/Bcfg2/Server/Info.py +++ b/src/lib/Bcfg2/Server/Info.py @@ -123,15 +123,6 @@ class InfoCmd(Bcfg2.Options.Subcommand): # pylint: disable=W0223 list(self.core.metadata.groups.keys())) -class Help(InfoCmd, Bcfg2.Options.HelpCommand): - """ Get help on a specific subcommand """ - def command_registry(self): - return self.core.commands - - def run(self, setup): - Bcfg2.Options.HelpCommand.run(self, setup) - - class Debug(InfoCmd): """ Shell out to a Python interpreter """ interpreters, default_interpreter = load_interpreters() @@ -805,15 +796,12 @@ if HAS_PROFILE: display_trace(prof) -class InfoCore(cmd.Cmd, - Bcfg2.Server.Core.Core, - Bcfg2.Options.CommandRegistry): +class InfoCore(cmd.Cmd, Bcfg2.Server.Core.Core): """Main class for bcfg2-info.""" def __init__(self): cmd.Cmd.__init__(self) Bcfg2.Server.Core.Core.__init__(self) - Bcfg2.Options.CommandRegistry.__init__(self) self.prompt = 'bcfg2-info> ' def get_locals(self): @@ -853,16 +841,17 @@ class InfoCore(cmd.Cmd, Bcfg2.Server.Core.Core.shutdown(self) -class CLI(object): +class CLI(Bcfg2.Options.CommandRegistry): """ The bcfg2-info CLI """ options = [Bcfg2.Options.BooleanOption("-p", "--profile", help="Profile")] def __init__(self): - Bcfg2.Options.register_commands(InfoCore, globals().values(), - parent=InfoCmd) + Bcfg2.Options.CommandRegistry.__init__(self) + self.register_commands(globals().values(), parent=InfoCmd) parser = Bcfg2.Options.get_parser( description="Inspect a running Bcfg2 server", components=[self, InfoCore]) + parser.add_options(self.subcommand_options) parser.parse() if Bcfg2.Options.setup.profile and HAS_PROFILE: @@ -874,11 +863,11 @@ class CLI(object): print("Profiling functionality not available.") self.core = InfoCore() - for command in self.core.commands.values(): + for command in self.commands.values(): command.core = self.core def run(self): """ Run bcfg2-info """ if Bcfg2.Options.setup.subcommand != 'help': self.core.run() - return self.core.runcommand() + return self.runcommand() diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/YumHelper.py b/src/lib/Bcfg2/Server/Plugins/Packages/YumHelper.py index 48304d26e..f26d6ba18 100644 --- a/src/lib/Bcfg2/Server/Plugins/Packages/YumHelper.py +++ b/src/lib/Bcfg2/Server/Plugins/Packages/YumHelper.py @@ -383,10 +383,10 @@ class CLI(Bcfg2.Options.CommandRegistry): def __init__(self): Bcfg2.Options.CommandRegistry.__init__(self) - Bcfg2.Options.register_commands(self.__class__, globals().values(), - parent=HelperSubcommand) + self.register_commands(globals().values(), parent=HelperSubcommand) parser = Bcfg2.Options.get_parser("Bcfg2 yum helper", components=[self]) + parser.add_options(self.subcommand_options) parser.parse() self.logger = logging.getLogger(parser.prog) -- cgit v1.2.3-1-g7c22