From 477c9c4119df5fd45c1129651922d238dccad8c9 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 From 5ceb76e562dc5aedae08910f65c2ff7cdf0c9ac1 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Tue, 14 Oct 2014 09:40:24 -0500 Subject: testsuite: skip nested exclusive option group test on py2.6 --- src/lib/Bcfg2/Options/OptionGroups.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/lib') diff --git a/src/lib/Bcfg2/Options/OptionGroups.py b/src/lib/Bcfg2/Options/OptionGroups.py index 426ee5dc0..49340ab36 100644 --- a/src/lib/Bcfg2/Options/OptionGroups.py +++ b/src/lib/Bcfg2/Options/OptionGroups.py @@ -70,8 +70,8 @@ class ExclusiveOptionGroup(_OptionContainer): 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, parser.add_mutually_exclusive_group(required=self.required)) class Subparser(_OptionContainer): -- cgit v1.2.3-1-g7c22 From dc9e04ecd8e7cedb4c1645044828442f264c8c9d Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 22 Oct 2014 07:58:01 -0500 Subject: fixed some places where plugin loading should fail silently --- src/lib/Bcfg2/Options/Actions.py | 8 ++++++-- src/lib/Bcfg2/Server/Plugins/Packages/__init__.py | 1 + src/lib/Bcfg2/Server/Plugins/Svn.py | 18 ++++++++++++------ 3 files changed, 19 insertions(+), 8 deletions(-) (limited to 'src/lib') diff --git a/src/lib/Bcfg2/Options/Actions.py b/src/lib/Bcfg2/Options/Actions.py index 2916cc194..854e6039d 100644 --- a/src/lib/Bcfg2/Options/Actions.py +++ b/src/lib/Bcfg2/Options/Actions.py @@ -113,9 +113,12 @@ class ComponentAction(FinalizableAction): try: return getattr(__import__(module, fromlist=[name]), name) except (AttributeError, ImportError): + msg = "Failed to load %s from %s: %s" % (name, module, + sys.exc_info()[1]) if not self.fail_silently: - print("Failed to load %s from %s: %s" % - (name, module, sys.exc_info()[1])) + print(msg) + else: + _debug(msg) return None def _load_component(self, name): @@ -181,3 +184,4 @@ class PluginsAction(ComponentAction): """ :class:`Bcfg2.Options.ComponentAction` subclass for loading Bcfg2 server plugins. """ bases = ['Bcfg2.Server.Plugins'] + fail_silently = True diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py b/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py index d11ac60fe..cb533f4f1 100644 --- a/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py +++ b/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py @@ -33,6 +33,7 @@ class PackagesBackendAction(Bcfg2.Options.ComponentAction): """ ComponentAction to load Packages backends """ bases = ['Bcfg2.Server.Plugins.Packages'] module = True + fail_silently = True class OnDemandDict(MutableMapping): diff --git a/src/lib/Bcfg2/Server/Plugins/Svn.py b/src/lib/Bcfg2/Server/Plugins/Svn.py index b752650f0..2ca518e53 100644 --- a/src/lib/Bcfg2/Server/Plugins/Svn.py +++ b/src/lib/Bcfg2/Server/Plugins/Svn.py @@ -17,12 +17,6 @@ except ImportError: class Svn(Bcfg2.Server.Plugin.Version): """Svn is a version plugin for dealing with Bcfg2 repos.""" options = Bcfg2.Server.Plugin.Version.options + [ - Bcfg2.Options.Option( - cf=("svn", "conflict_resolution"), dest="svn_conflict_resolution", - type=lambda v: v.replace("-", "_"), - choices=dir(pysvn.wc_conflict_choice), # pylint: disable=E1101 - default=pysvn.wc_conflict_choice.postpone, # pylint: disable=E1101 - help="SVN conflict resolution method"), Bcfg2.Options.Option( cf=("svn", "user"), dest="svn_user", help="SVN username"), Bcfg2.Options.Option( @@ -31,6 +25,18 @@ class Svn(Bcfg2.Server.Plugin.Version): cf=("svn", "always_trust"), dest="svn_trust_ssl", help="Always trust SSL certs from SVN server")] + if HAS_SVN: + options.append( + Bcfg2.Options.Option( + cf=("svn", "conflict_resolution"), + dest="svn_conflict_resolution", + type=lambda v: v.replace("-", "_"), + # pylint: disable=E1101 + choices=dir(pysvn.wc_conflict_choice), + default=pysvn.wc_conflict_choice.postpone, + # pylint: enable=E1101 + help="SVN conflict resolution method")) + __author__ = 'bcfg-dev@mcs.anl.gov' __vcs_metadata_path__ = ".svn" if HAS_SVN: -- cgit v1.2.3-1-g7c22 From 987023ba91d38a58c2fd4269bdfd0bf40648b7de Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 22 Oct 2014 08:07:28 -0500 Subject: call shutdown on subcommand registries --- src/lib/Bcfg2/Server/Admin.py | 7 +++++-- src/lib/Bcfg2/Server/Info.py | 10 ++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) (limited to 'src/lib') diff --git a/src/lib/Bcfg2/Server/Admin.py b/src/lib/Bcfg2/Server/Admin.py index b6dded175..ef7741880 100644 --- a/src/lib/Bcfg2/Server/Admin.py +++ b/src/lib/Bcfg2/Server/Admin.py @@ -1197,5 +1197,8 @@ class CLI(Bcfg2.Options.CommandRegistry): def run(self): """ Run bcfg2-admin """ - self.commands[Bcfg2.Options.setup.subcommand].setup() - return self.runcommand() + try: + self.commands[Bcfg2.Options.setup.subcommand].setup() + return self.runcommand() + finally: + self.shutdown() diff --git a/src/lib/Bcfg2/Server/Info.py b/src/lib/Bcfg2/Server/Info.py index 0baa29a9f..da2312cc8 100644 --- a/src/lib/Bcfg2/Server/Info.py +++ b/src/lib/Bcfg2/Server/Info.py @@ -837,7 +837,6 @@ class InfoCore(cmd.Cmd, Bcfg2.Server.Core.Core): pass def shutdown(self): - Bcfg2.Options.CommandRegistry.shutdown(self) Bcfg2.Server.Core.Core.shutdown(self) @@ -868,6 +867,9 @@ class CLI(Bcfg2.Options.CommandRegistry): def run(self): """ Run bcfg2-info """ - if Bcfg2.Options.setup.subcommand != 'help': - self.core.run() - return self.runcommand() + try: + if Bcfg2.Options.setup.subcommand != 'help': + self.core.run() + return self.runcommand() + finally: + self.shutdown() -- cgit v1.2.3-1-g7c22 From 8318378b1ee2f6c0a9e5446036c7409228545e16 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 22 Oct 2014 11:02:51 -0500 Subject: testsuite: better debug capturing for options tests --- src/lib/Bcfg2/Options/Options.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'src/lib') diff --git a/src/lib/Bcfg2/Options/Options.py b/src/lib/Bcfg2/Options/Options.py index 4efd76929..bd1a72fc7 100644 --- a/src/lib/Bcfg2/Options/Options.py +++ b/src/lib/Bcfg2/Options/Options.py @@ -17,14 +17,18 @@ from Bcfg2.Compat import ConfigParser __all__ = ["Option", "BooleanOption", "PathOption", "PositionalArgument", "_debug"] +unit_test = False + def _debug(msg): """ Option parsing happens before verbose/debug have been set -- 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').lower() in ["true", "yes", - "on", "1"]: + if unit_test: + print("DEBUG: %s" % msg) + elif os.environ.get('BCFG2_OPTIONS_DEBUG', '0').lower() in ["true", "yes", + "on", "1"]: sys.stderr.write("%s\n" % msg) -- cgit v1.2.3-1-g7c22 From 72201ddac165e45da09521b77660b2e155ca36cd Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 22 Oct 2014 11:03:17 -0500 Subject: DBSettings: fix up in database name --- src/lib/Bcfg2/DBSettings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/lib') diff --git a/src/lib/Bcfg2/DBSettings.py b/src/lib/Bcfg2/DBSettings.py index b817ecb94..ec27306a4 100644 --- a/src/lib/Bcfg2/DBSettings.py +++ b/src/lib/Bcfg2/DBSettings.py @@ -212,7 +212,7 @@ class _OptionContainer(object): Bcfg2.Options.Option( cf=('database', 'engine'), default='sqlite3', help='Database engine', dest='db_engine'), - Bcfg2.Options.Option( + Bcfg2.Options.PathOption( cf=('database', 'name'), default='/etc/bcfg2.sqlite', help="Database name", dest="db_name"), Bcfg2.Options.Option( -- cgit v1.2.3-1-g7c22 From 4ec92cb9e7d1eb2f90d36e5255ee8814ca0a8513 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 22 Oct 2014 11:03:48 -0500 Subject: Options: ensure macros are always fixed up This fixes several cases in which macros would not be properly processed: options that are not added to the parser yet when early options are parsed; and config file options whose default value is used. --- src/lib/Bcfg2/Options/Options.py | 59 +++++++++++++++++++++--------- src/lib/Bcfg2/Options/Parser.py | 77 ++++++++++++++++++++++++++++------------ 2 files changed, 96 insertions(+), 40 deletions(-) (limited to 'src/lib') diff --git a/src/lib/Bcfg2/Options/Options.py b/src/lib/Bcfg2/Options/Options.py index bd1a72fc7..36148c279 100644 --- a/src/lib/Bcfg2/Options/Options.py +++ b/src/lib/Bcfg2/Options/Options.py @@ -17,7 +17,7 @@ from Bcfg2.Compat import ConfigParser __all__ = ["Option", "BooleanOption", "PathOption", "PositionalArgument", "_debug"] -unit_test = False +unit_test = False # pylint: disable=C0103 def _debug(msg): @@ -46,7 +46,7 @@ def _get_action_class(action_name): on. So we just instantiate a dummy parser, add a dummy argument, and determine the class that way. """ if (isinstance(action_name, type) and - issubclass(action_name, argparse.Action)): + issubclass(action_name, argparse.Action)): return action_name if action_name not in _action_map: action = argparse.ArgumentParser().add_argument(action_name, @@ -159,9 +159,10 @@ class Option(object): sources.append("%s.%s" % self.cf) if self.env: sources.append("$" + self.env) - spec = ["sources=%s" % sources, "default=%s" % self.default] - spec.append("%d parsers" % (len(self.parsers))) - return 'Option(%s: %s)' % (self.dest, ", ".join(spec)) + spec = ["sources=%s" % sources, "default=%s" % self.default, + "%d parsers" % len(self.parsers)] + return '%s(%s: %s)' % (self.__class__.__name__, + self.dest, ", ".join(spec)) def list_options(self): """ List options contained in this option. This exists to @@ -228,7 +229,7 @@ class Option(object): 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)) + _debug("Getting value of %s from config file(s): %s" % (self, rv)) return rv def get_config_value(self, cfp): @@ -326,12 +327,11 @@ class Option(object): class PathOption(Option): - """ Shortcut for options that expect a path argument. Uses - :meth:`Bcfg2.Options.Types.path` to transform the argument into a - canonical path. + """Shortcut for options that expect a path argument. - The type of a path option can also be overridden to return a - file-like object. For example: + Uses :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 a file-like object. For example: .. code-block:: python @@ -339,25 +339,50 @@ class PathOption(Option): Bcfg2.Options.PathOption( "--input", type=argparse.FileType('r'), help="The input file")] + + PathOptions also do translation of ```` macros on the + fly. It's done this way instead of just fixing up all values at + the end of parsing because macro expansion needs to be done before + path canonicalization and other stuff. """ + repository = None def __init__(self, *args, **kwargs): 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 + if hasattr(early_opts, "repository"): + if self.__class__.repository is None: + _debug("Setting repository to %s for PathOptions" % + early_opts.repository) + self.__class__.repository = early_opts.repository + else: + _debug("Repository is already set for %s" % self.__class__) + + def _get_default(self): + """ Getter for the ``default`` property """ + if self.__class__.repository is None or self._default is None: + return self._default + else: + return self._default.replace("", + self.__class__.repository) + + default = property(_get_default, Option._set_default) def _type(self, value): """Type function that fixes up macros.""" - if self.repository is None: - rv = value + if self.__class__.repository is None: + _debug("Cannot fix up macros yet for %s" % self) + return value else: - rv = value.replace("", self.repository) - return self._original_type(Types.path(rv)) + rv = self._original_type(Types.path( + value.replace("", self.__class__.repository))) + _debug("Fixing up macros in %s: %s -> %s" % + (self, value, rv)) + return rv class _BooleanOptionAction(argparse.Action): diff --git a/src/lib/Bcfg2/Options/Parser.py b/src/lib/Bcfg2/Options/Parser.py index ec470f8d3..6715d90f9 100644 --- a/src/lib/Bcfg2/Options/Parser.py +++ b/src/lib/Bcfg2/Options/Parser.py @@ -168,6 +168,8 @@ class Parser(argparse.ArgumentParser): (opt, value)) action(self, self.namespace, value) else: + _debug("Setting config file-only option %s to %s" % + (opt, value)) setattr(self.namespace, opt.dest, value) def _finalize(self): @@ -191,6 +193,53 @@ class Parser(argparse.ArgumentParser): _debug("Deleting %s" % attr) delattr(self.namespace, attr) + def _parse_early_options(self): + """Parse early options. + + Early options are options that need to be parsed before other + options for some reason. These fall into two basic cases: + + 1. Database options, which need to be parsed so that Django + modules can be imported, since Django configuration is all + done at import-time; + 2. The repository (``-Q``) option, so that ```` + macros in other options can be resolved. + """ + _debug("Option parsing phase 2: Parse early options") + early_opts = argparse.Namespace() + early_parser = Parser(add_help=False, namespace=early_opts, + early=True) + + # add the repo option so we can resolve + # macros + early_parser.add_options([repository]) + + early_components = [] + for component in self.components: + if getattr(component, "parse_first", False): + early_components.append(component) + early_parser.add_component(component) + early_parser.parse(self.argv) + + _debug("Fixing up macros in early options") + for attr_name in dir(early_opts): + if not attr_name.startswith("_"): + attr = getattr(early_opts, attr_name) + if hasattr(attr, "replace"): + setattr(early_opts, attr_name, + attr.replace("", + early_opts.repository)) + + _debug("Early parsing complete, calling hooks") + for component in early_components: + if hasattr(component, "component_parsed_hook"): + _debug("Calling component_parsed_hook on %s" % component) + getattr(component, "component_parsed_hook")(early_opts) + _debug("Calling early parsing hooks; early options: %s" % + early_opts) + for option in self.option_list: + option.early_parsing_hook(early_opts) + def add_config_file(self, dest, cfile, reparse=True): """ Add a config file, which triggers a full reparse of all options. """ @@ -207,10 +256,11 @@ class Parser(argparse.ArgumentParser): def reparse(self, argv=None): """ Reparse options after they have already been parsed. - :param argv: The argument list to parse. By default, + :param argv: The argument list to parse. By default, :attr:`Bcfg2.Options.Parser.argv` is reused. (I.e., the argument list that was initially - parsed.) :type argv: list + parsed.) + :type argv: list """ _debug("Reparsing all options") self._reset_namespace() @@ -249,26 +299,7 @@ class Parser(argparse.ArgumentParser): # phase 2: re-parse command line for early options; currently, # that's database 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) - # add the repo option so we can resolve - # macros - early_parser.add_options([repository]) - early_components = [] - for component in self.components: - if getattr(component, "parse_first", False): - early_components.append(component) - early_parser.add_component(component) - early_parser.parse(self.argv) - _debug("Early parsing complete, calling hooks") - for component in early_components: - 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) + self._parse_early_options() else: _debug("Skipping parsing phase 2 in early mode") @@ -299,13 +330,13 @@ class Parser(argparse.ArgumentParser): remaining = [] while not self.parsed: self.parsed = True - self._set_defaults_from_config() _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._set_defaults_from_config() self._parse_config_options() self._finalize() if len(remaining) and not self._early: -- cgit v1.2.3-1-g7c22 From 3d26097342ecbda755135dc3cf39517d75c027aa Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 22 Oct 2014 13:02:35 -0500 Subject: Options: fix path canonicalization and file-like objects This fixes canonicalizing PathOption values when the default value of a config file-only option is used. It also fixes PathOptions that get a file-like object instead of a filename string. --- src/lib/Bcfg2/Options/Options.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) (limited to 'src/lib') diff --git a/src/lib/Bcfg2/Options/Options.py b/src/lib/Bcfg2/Options/Options.py index 36148c279..f1a201f1e 100644 --- a/src/lib/Bcfg2/Options/Options.py +++ b/src/lib/Bcfg2/Options/Options.py @@ -364,25 +364,20 @@ class PathOption(Option): def _get_default(self): """ Getter for the ``default`` property """ - if self.__class__.repository is None or self._default is None: + if not hasattr(self._default, "replace"): return self._default else: - return self._default.replace("", - self.__class__.repository) + return self._type(self._default) default = property(_get_default, Option._set_default) def _type(self, value): """Type function that fixes up macros.""" if self.__class__.repository is None: - _debug("Cannot fix up macros yet for %s" % self) return value else: - rv = self._original_type(Types.path( + return self._original_type(Types.path( value.replace("", self.__class__.repository))) - _debug("Fixing up macros in %s: %s -> %s" % - (self, value, rv)) - return rv class _BooleanOptionAction(argparse.Action): -- cgit v1.2.3-1-g7c22 From 75225b07acbda8dab8215a626088ba19739f73cb Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Mon, 10 Nov 2014 10:23:30 -0600 Subject: Options: gather as much data from config file first --- src/lib/Bcfg2/Options/Parser.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'src/lib') diff --git a/src/lib/Bcfg2/Options/Parser.py b/src/lib/Bcfg2/Options/Parser.py index 6715d90f9..c846e8093 100644 --- a/src/lib/Bcfg2/Options/Parser.py +++ b/src/lib/Bcfg2/Options/Parser.py @@ -325,8 +325,16 @@ class Parser(argparse.ArgumentParser): # _parse_config_options is called, all config file options will get set # to their hardcoded defaults. This process defines the options in the # namespace and _parse_config_options will never look at them again. - self._set_defaults_from_config() - self._parse_config_options() + # + # we have to do the parsing in two loops: first, we squeeze as + # much data out of the config file as we can to ensure that + # all config file settings are read before we use any default + # values. then we can start looking at the command line. + while not self.parsed: + self.parsed = True + self._set_defaults_from_config() + self._parse_config_options() + self.parsed = False remaining = [] while not self.parsed: self.parsed = True -- cgit v1.2.3-1-g7c22 From 669a8fce985164632b41bcac8fef3f52223bac0e Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Mon, 10 Nov 2014 10:32:10 -0600 Subject: Options: further command registry fixes This updates the documentation for some of the recent changes to subcommand handling, and ensures that the server core is shut down by bcfg2-info. --- src/lib/Bcfg2/Server/Info.py | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/lib') diff --git a/src/lib/Bcfg2/Server/Info.py b/src/lib/Bcfg2/Server/Info.py index da2312cc8..6af561089 100644 --- a/src/lib/Bcfg2/Server/Info.py +++ b/src/lib/Bcfg2/Server/Info.py @@ -873,3 +873,7 @@ class CLI(Bcfg2.Options.CommandRegistry): return self.runcommand() finally: self.shutdown() + + def shutdown(self): + Bcfg2.Options.CommandRegistry.shutdown(self) + self.core.shutdown() -- cgit v1.2.3-1-g7c22 From aae0bafb03ee6e986c51436ec749bbadd97a2f7f Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Mon, 10 Nov 2014 11:42:42 -0600 Subject: Options: Fixed non-path database name parsing The database name is sometimes a path (SQLite) and sometimes not (MySQL, PostgreSQL). This introduces a new Option type, RepositoryMacroOption, that expands macros without canonicalizing the path, so SQLite users can use in their settings but MySQL users' database name settings will not be destroyed by path canonicalization. The unfortunate downside is that SQLite users can't use ~ in their database name. --- src/lib/Bcfg2/DBSettings.py | 2 +- src/lib/Bcfg2/Options/Options.py | 63 ++++++++++++++++++++++++++-------------- 2 files changed, 42 insertions(+), 23 deletions(-) (limited to 'src/lib') diff --git a/src/lib/Bcfg2/DBSettings.py b/src/lib/Bcfg2/DBSettings.py index ec27306a4..f5b5d16aa 100644 --- a/src/lib/Bcfg2/DBSettings.py +++ b/src/lib/Bcfg2/DBSettings.py @@ -212,7 +212,7 @@ class _OptionContainer(object): Bcfg2.Options.Option( cf=('database', 'engine'), default='sqlite3', help='Database engine', dest='db_engine'), - Bcfg2.Options.PathOption( + Bcfg2.Options.RepositoryMacroOption( cf=('database', 'name'), default='/etc/bcfg2.sqlite', help="Database name", dest="db_name"), Bcfg2.Options.Option( diff --git a/src/lib/Bcfg2/Options/Options.py b/src/lib/Bcfg2/Options/Options.py index f1a201f1e..752e01b4e 100644 --- a/src/lib/Bcfg2/Options/Options.py +++ b/src/lib/Bcfg2/Options/Options.py @@ -14,8 +14,8 @@ from Bcfg2.Options import Types from Bcfg2.Compat import ConfigParser -__all__ = ["Option", "BooleanOption", "PathOption", "PositionalArgument", - "_debug"] +__all__ = ["Option", "BooleanOption", "RepositoryMacroOption", "PathOption", + "PositionalArgument", "_debug"] unit_test = False # pylint: disable=C0103 @@ -326,24 +326,13 @@ class Option(object): (self, parser)) -class PathOption(Option): - """Shortcut for options that expect a path argument. - - Uses :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 a file-like object. For example: - - .. code-block:: python - - options = [ - Bcfg2.Options.PathOption( - "--input", type=argparse.FileType('r'), - help="The input file")] +class RepositoryMacroOption(Option): + """Option that does translation of ```` macros. - PathOptions also do translation of ```` macros on the - fly. It's done this way instead of just fixing up all values at - the end of parsing because macro expansion needs to be done before - path canonicalization and other stuff. + Macro translation is done on the fly instead of just fixing up all + values at the end of parsing because macro expansion needs to be + done before path canonicalization for + :class:`Bcfg2.Options.Options.PathOption`. """ repository = None @@ -356,8 +345,8 @@ class PathOption(Option): def early_parsing_hook(self, early_opts): if hasattr(early_opts, "repository"): if self.__class__.repository is None: - _debug("Setting repository to %s for PathOptions" % - early_opts.repository) + _debug("Setting repository to %s for %s" % + (early_opts.repository, self.__class__.__name__)) self.__class__.repository = early_opts.repository else: _debug("Repository is already set for %s" % self.__class__) @@ -371,15 +360,45 @@ class PathOption(Option): default = property(_get_default, Option._set_default) + def transform_value(self, value): + """transform the value after macro expansion. + + this can be overridden to further transform the value set by + the user *after* macros are expanded, but before the user's + ``type`` function is applied. principally exists for + PathOption to canonicalize the path. + """ + return value + def _type(self, value): """Type function that fixes up macros.""" if self.__class__.repository is None: return value else: - return self._original_type(Types.path( + return self._original_type(self.transform_value( value.replace("", self.__class__.repository))) +class PathOption(RepositoryMacroOption): + """Shortcut for options that expect a path argument. + + Uses :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 a file-like object. For example: + + .. code-block:: python + + options = [ + Bcfg2.Options.PathOption( + "--input", type=argparse.FileType('r'), + help="The input file")] + + PathOptions also do translation of ```` macros. + """ + def transform_value(self, value): + return Types.path(value) + + class _BooleanOptionAction(argparse.Action): """BooleanOptionAction sets a boolean value. -- cgit v1.2.3-1-g7c22 From 30634d07d5489f260f37cc86d150315f02c40865 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Tue, 11 Nov 2014 13:22:30 -0600 Subject: removed duplicate plugins option --- src/lib/Bcfg2/Server/models.py | 30 +++++------------------------- 1 file changed, 5 insertions(+), 25 deletions(-) (limited to 'src/lib') diff --git a/src/lib/Bcfg2/Server/models.py b/src/lib/Bcfg2/Server/models.py index 7150c245a..8d6642a25 100644 --- a/src/lib/Bcfg2/Server/models.py +++ b/src/lib/Bcfg2/Server/models.py @@ -4,39 +4,19 @@ import sys import logging import Bcfg2.Options import Bcfg2.Server.Plugins -from Bcfg2.Compat import walk_packages -LOGGER = logging.getLogger('Bcfg2.Server.models') +LOGGER = logging.getLogger(__name__) MODELS = [] -def _get_all_plugins(): - rv = [] - for submodule in walk_packages(path=Bcfg2.Server.Plugins.__path__, - prefix="Bcfg2.Server.Plugins."): - module = submodule[1].rsplit('.', 1)[-1] - if submodule[1] == "Bcfg2.Server.Plugins.%s" % module: - # we only include direct children of - # Bcfg2.Server.Plugins -- e.g., all_plugins should - # include Bcfg2.Server.Plugins.Cfg, but not - # Bcfg2.Server.Plugins.Cfg.CfgInfoXML - rv.append(module) - return rv - - -_ALL_PLUGINS = _get_all_plugins() - - class _OptionContainer(object): + """Options for Bcfg2 database models.""" + # we want to provide a different default plugin list -- # namely, _all_ plugins, so that the database is guaranteed to # work, even if /etc/bcfg2.conf isn't set up properly - options = [ - Bcfg2.Options.Option( - cf=('server', 'plugins'), type=Bcfg2.Options.Types.comma_list, - default=_ALL_PLUGINS, dest="models_plugins", - action=Bcfg2.Options.PluginsAction)] + options = [Bcfg2.Options.Common.plugins] @staticmethod def options_parsed_hook(): @@ -58,7 +38,7 @@ def load_models(plugins=None): global MODELS if not plugins: - plugins = Bcfg2.Options.setup.models_plugins + plugins = Bcfg2.Options.setup.plugins if MODELS: # load_models() has been called once, so first unload all of -- cgit v1.2.3-1-g7c22