From e9bc14845afadb75b25233b683b5a0564dc52636 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Sat, 29 Jul 2017 17:45:36 +0200 Subject: [PATCH] [enh] Make ActionsMap._construct_parser code more readable (#144) * [mod] autopep8 * [mod] remove useless try/except * [mod] renaming + comment * [mod] remove debug print * [mod] more renaming * [doc] explicit comment * [mod] more renaming * [mod] more renaming and comments * [mod] more renaming * [doc] usefull comment * [mod] more renaming * [mod] this is always true * [mod] remove conditions that are always True * [mod] more renaming * [doc] add a comment from the docstring * [mod] remove useless exception * [mod] remove useless () * [mod] style * [mod] it's better to fail than to silencly ignore a typo * [mod] more renaming * [doc] add a comment to avoid looking at yet-another-file * [mod] renaming, make method name match reality and had a method for code readability * [mod] remove one indirection level * [mod] we always have actions * [doc] add more comments * [mod] simplify code again * [mod] case never happen * [mod] delegate responsability to class to make code more readable * [mod] finish moving _add_arguments down to parsers * [mod] invert conditions to make things more readable * [mod] style * [mod] remove useless exception * [mod] simplify code --- moulinette/actionsmap.py | 118 +++++++++++------------------- moulinette/interfaces/__init__.py | 21 ++++++ moulinette/interfaces/api.py | 23 +++++- moulinette/interfaces/cli.py | 23 ++++-- 4 files changed, 99 insertions(+), 86 deletions(-) diff --git a/moulinette/actionsmap.py b/moulinette/actionsmap.py index 3fe8cd03..79fd7808 100644 --- a/moulinette/actionsmap.py +++ b/moulinette/actionsmap.py @@ -562,94 +562,64 @@ class ActionsMap(object): """ # Get extra parameters - if not self.use_cache: - validate_extra = True - else: + if self.use_cache: validate_extra = False - - # Add arguments to the parser - def _add_arguments(tid, parser, arguments): - for argn, argp in arguments.items(): - names = top_parser.format_arg_names(str(argn), - argp.pop('full', None)) - try: - argp['type'] = eval(argp['type']) - except: - pass - - try: - extra = argp.pop('extra') - arg_dest = (parser.add_argument(*names, **argp)).dest - self.extraparser.add_argument(tid, arg_dest, extra, - validate_extra) - except KeyError: - # No extra parameters - parser.add_argument(*names, **argp) + else: + validate_extra = True # Instantiate parser + # + # this either returns: + # * moulinette.interfaces.cli.ActionsMapParser + # * moulinette.interfaces.api.ActionsMapParser top_parser = self.parser_class(**kwargs) - # Iterate over actions map namespaces - for n, actionsmap in actionsmaps.items(): + # namespace, actionsmap is a tuple where: + # + # * namespace define the top "name", for us it will always be + # "yunohost" and there well be only this one + # * actionsmap is the actual actionsmap that we care about + for namespace, actionsmap in actionsmaps.items(): # Retrieve global parameters _global = actionsmap.pop('_global', {}) - # -- Parse global configuration - if 'configuration' in _global: - # Set global configuration - top_parser.set_global_conf(_global['configuration']) + # Set the global configuration to use for the parser. + top_parser.set_global_conf(_global['configuration']) - # -- Parse global arguments - if 'arguments' in _global: - try: - # Get global arguments parser - parser = top_parser.add_global_parser() - except AttributeError: - # No parser for global arguments - pass - else: - # Add arguments - _add_arguments(GLOBAL_SECTION, parser, - _global['arguments']) + if top_parser.has_global_parser(): + top_parser.add_global_arguments(_global['arguments']) - # -- Parse categories - for cn, cp in actionsmap.items(): - try: - actions = cp.pop('actions') - except KeyError: - # Invalid category without actions - logger.warning("no actions found in category '%s' in " - "namespace '%s'", cn, n) - continue + # category_name is stuff like "user", "domain", "hooks"... + # category_values is the values of this category (like actions) + for category_name, category_values in actionsmap.items(): + actions = category_values.pop('actions') # Get category parser - cat_parser = top_parser.add_category_parser(cn, **cp) + category_parser = top_parser.add_category_parser(category_name, + **category_values) - # -- Parse actions - for an, ap in actions.items(): - args = ap.pop('arguments', {}) - tid = (n, cn, an) - try: - conf = ap.pop('configuration') - _set_conf = lambda p: p.set_conf(tid, conf) - except KeyError: - # No action configuration - _set_conf = lambda p: False + # action_name is like "list" of "domain list" + # action_options are the values + for action_name, action_options in actions.items(): + arguments = action_options.pop('arguments', {}) + tid = (namespace, category_name, action_name) - try: - # Get action parser - a_parser = cat_parser.add_action_parser(an, tid, **ap) - except AttributeError: - # No parser for the action + # Get action parser + action_parser = category_parser.add_action_parser(action_name, + tid, + **action_options) + + if action_parser is None: # No parser for the action continue - except ValueError as e: - logger.warning("cannot add action (%s, %s, %s): %s", - n, cn, an, e) - continue - else: - # Store action identifier and add arguments - a_parser.set_defaults(_tid=tid) - _add_arguments(tid, a_parser, args) - _set_conf(cat_parser) + + # Store action identifier and add arguments + action_parser.set_defaults(_tid=tid) + action_parser.add_arguments(arguments, + extraparser=self.extraparser, + format_arg_names=top_parser.format_arg_names, + validate_extra=validate_extra) + + if 'configuration' in action_options: + category_parser.set_conf(tid, action_options['configuration']) return top_parser diff --git a/moulinette/interfaces/__init__.py b/moulinette/interfaces/__init__.py index 01e57bcc..4eace2bf 100644 --- a/moulinette/interfaces/__init__.py +++ b/moulinette/interfaces/__init__.py @@ -72,6 +72,9 @@ class BaseActionsMapParser(object): raise NotImplementedError("derived class '%s' must override this method" % self.__class__.__name__) + def has_global_parser(self): + return False + def add_global_parser(self, **kwargs): """Add a parser for global arguments @@ -535,6 +538,24 @@ class ExtendedArgumentParser(argparse.ArgumentParser): queue = list() return queue + def add_arguments(self, arguments, extraparser, format_arg_names=None, validate_extra=True): + for argument_name, argument_options in arguments.items(): + # will adapt arguments name for cli or api context + names = format_arg_names(str(argument_name), + argument_options.pop('full', None)) + + if "type" in argument_options: + argument_options['type'] = eval(argument_options['type']) + + if "extra" in argument_options: + extra = argument_options.pop('extra') + argument_dest = self.add_argument(*names, **argument_options).dest + extraparser.add_argument(self.get_default("_tid"), + argument_dest, extra, validate_extra) + continue + + self.add_argument(*names, **argument_options) + def _get_nargs_pattern(self, action): if action.nargs == argparse.PARSER and not action.required: return '([-AO]*)' diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py index 4901cafd..e86cf876 100644 --- a/moulinette/interfaces/api.py +++ b/moulinette/interfaces/api.py @@ -80,6 +80,24 @@ class _HTTPArgumentParser(object): def get_default(self, dest): return self._parser.get_default(dest) + def add_arguments(self, arguments, extraparser, format_arg_names=None, validate_extra=True): + for argument_name, argument_options in arguments.items(): + # will adapt arguments name for cli or api context + names = format_arg_names(str(argument_name), + argument_options.pop('full', None)) + + if "type" in argument_options: + argument_options['type'] = eval(argument_options['type']) + + if "extra" in argument_options: + extra = argument_options.pop('extra') + argument_dest = self.add_argument(*names, **argument_options).dest + extraparser.add_argument(self.get_default("_tid"), + argument_dest, extra, validate_extra) + continue + + self.add_argument(*names, **argument_options) + def add_argument(self, *args, **kwargs): action = self._parser.add_argument(*args, **kwargs) @@ -557,9 +575,6 @@ class ActionsMapParser(BaseActionsMapParser): return [name.replace('--', '@', 1)] return [name.replace('-', '@', 1)] - def add_global_parser(self, **kwargs): - raise AttributeError("global arguments are not managed") - def add_category_parser(self, name, **kwargs): return self @@ -590,7 +605,7 @@ class ActionsMapParser(BaseActionsMapParser): if len(keys) == 0: raise ValueError("no valid api route found") else: - raise AttributeError("no api route for action '%s'" % name) + return None # Create and append parser parser = _HTTPArgumentParser() diff --git a/moulinette/interfaces/cli.py b/moulinette/interfaces/cli.py index ada6697d..0ca171e3 100644 --- a/moulinette/interfaces/cli.py +++ b/moulinette/interfaces/cli.py @@ -231,14 +231,15 @@ class ActionsMapParser(BaseActionsMapParser): self._parser = parser or ExtendedArgumentParser() self._subparsers = self._parser.add_subparsers(**subparser_kwargs) - self._global_parser = parent._global_parser if parent else None + self.global_parser = parent.global_parser if parent else None if top_parser: + self.global_parser = self._parser.add_argument_group("global arguments") + # Append each top parser action to the global group - glob = self.add_global_parser() for action in top_parser._actions: action.dest = SUPPRESS - glob._add_action(action) + self.global_parser._add_action(action) # Implement virtual properties @@ -252,11 +253,8 @@ class ActionsMapParser(BaseActionsMapParser): return [name, full] return [name] - def add_global_parser(self, **kwargs): - if not self._global_parser: - self._global_parser = self._parser.add_argument_group( - "global arguments") - return self._global_parser + def has_global_parser(self): + return True def add_category_parser(self, name, category_help=None, **kwargs): """Add a parser for a category @@ -290,6 +288,15 @@ class ActionsMapParser(BaseActionsMapParser): deprecated=deprecated, deprecated_alias=deprecated_alias) + def add_global_arguments(self, arguments): + for argument_name, argument_options in arguments.items(): + # will adapt arguments name for cli or api context + names = self.format_arg_names(str(argument_name), + argument_options.pop('full', None)) + + self.global_parser.add_argument(*names, **argument_options) + + def parse_args(self, args, **kwargs): try: ret = self._parser.parse_args(args)