[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
This commit is contained in:
Laurent Peuch 2017-07-29 17:45:36 +02:00 committed by Alexandre Aubin
parent 8960b1a610
commit e9bc14845a
4 changed files with 99 additions and 86 deletions

View file

@ -562,94 +562,64 @@ class ActionsMap(object):
""" """
# Get extra parameters # Get extra parameters
if not self.use_cache: if self.use_cache:
validate_extra = True
else:
validate_extra = False validate_extra = False
else:
# Add arguments to the parser validate_extra = True
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)
# Instantiate parser # Instantiate parser
#
# this either returns:
# * moulinette.interfaces.cli.ActionsMapParser
# * moulinette.interfaces.api.ActionsMapParser
top_parser = self.parser_class(**kwargs) top_parser = self.parser_class(**kwargs)
# Iterate over actions map namespaces # namespace, actionsmap is a tuple where:
for n, actionsmap in actionsmaps.items(): #
# * 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 # Retrieve global parameters
_global = actionsmap.pop('_global', {}) _global = actionsmap.pop('_global', {})
# -- Parse global configuration # Set the global configuration to use for the parser.
if 'configuration' in _global: top_parser.set_global_conf(_global['configuration'])
# Set global configuration
top_parser.set_global_conf(_global['configuration'])
# -- Parse global arguments if top_parser.has_global_parser():
if 'arguments' in _global: top_parser.add_global_arguments(_global['arguments'])
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'])
# -- Parse categories # category_name is stuff like "user", "domain", "hooks"...
for cn, cp in actionsmap.items(): # category_values is the values of this category (like actions)
try: for category_name, category_values in actionsmap.items():
actions = cp.pop('actions') actions = category_values.pop('actions')
except KeyError:
# Invalid category without actions
logger.warning("no actions found in category '%s' in "
"namespace '%s'", cn, n)
continue
# Get category parser # 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 # action_name is like "list" of "domain list"
for an, ap in actions.items(): # action_options are the values
args = ap.pop('arguments', {}) for action_name, action_options in actions.items():
tid = (n, cn, an) arguments = action_options.pop('arguments', {})
try: tid = (namespace, category_name, action_name)
conf = ap.pop('configuration')
_set_conf = lambda p: p.set_conf(tid, conf)
except KeyError:
# No action configuration
_set_conf = lambda p: False
try: # Get action parser
# Get action parser action_parser = category_parser.add_action_parser(action_name,
a_parser = cat_parser.add_action_parser(an, tid, **ap) tid,
except AttributeError: **action_options)
# No parser for the action
if action_parser is None: # No parser for the action
continue continue
except ValueError as e:
logger.warning("cannot add action (%s, %s, %s): %s", # Store action identifier and add arguments
n, cn, an, e) action_parser.set_defaults(_tid=tid)
continue action_parser.add_arguments(arguments,
else: extraparser=self.extraparser,
# Store action identifier and add arguments format_arg_names=top_parser.format_arg_names,
a_parser.set_defaults(_tid=tid) validate_extra=validate_extra)
_add_arguments(tid, a_parser, args)
_set_conf(cat_parser) if 'configuration' in action_options:
category_parser.set_conf(tid, action_options['configuration'])
return top_parser return top_parser

View file

@ -72,6 +72,9 @@ class BaseActionsMapParser(object):
raise NotImplementedError("derived class '%s' must override this method" % raise NotImplementedError("derived class '%s' must override this method" %
self.__class__.__name__) self.__class__.__name__)
def has_global_parser(self):
return False
def add_global_parser(self, **kwargs): def add_global_parser(self, **kwargs):
"""Add a parser for global arguments """Add a parser for global arguments
@ -535,6 +538,24 @@ class ExtendedArgumentParser(argparse.ArgumentParser):
queue = list() queue = list()
return queue 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): def _get_nargs_pattern(self, action):
if action.nargs == argparse.PARSER and not action.required: if action.nargs == argparse.PARSER and not action.required:
return '([-AO]*)' return '([-AO]*)'

View file

@ -80,6 +80,24 @@ class _HTTPArgumentParser(object):
def get_default(self, dest): def get_default(self, dest):
return self._parser.get_default(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): def add_argument(self, *args, **kwargs):
action = self._parser.add_argument(*args, **kwargs) action = self._parser.add_argument(*args, **kwargs)
@ -557,9 +575,6 @@ class ActionsMapParser(BaseActionsMapParser):
return [name.replace('--', '@', 1)] return [name.replace('--', '@', 1)]
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): def add_category_parser(self, name, **kwargs):
return self return self
@ -590,7 +605,7 @@ class ActionsMapParser(BaseActionsMapParser):
if len(keys) == 0: if len(keys) == 0:
raise ValueError("no valid api route found") raise ValueError("no valid api route found")
else: else:
raise AttributeError("no api route for action '%s'" % name) return None
# Create and append parser # Create and append parser
parser = _HTTPArgumentParser() parser = _HTTPArgumentParser()

View file

@ -231,14 +231,15 @@ class ActionsMapParser(BaseActionsMapParser):
self._parser = parser or ExtendedArgumentParser() self._parser = parser or ExtendedArgumentParser()
self._subparsers = self._parser.add_subparsers(**subparser_kwargs) 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: if top_parser:
self.global_parser = self._parser.add_argument_group("global arguments")
# Append each top parser action to the global group # Append each top parser action to the global group
glob = self.add_global_parser()
for action in top_parser._actions: for action in top_parser._actions:
action.dest = SUPPRESS action.dest = SUPPRESS
glob._add_action(action) self.global_parser._add_action(action)
# Implement virtual properties # Implement virtual properties
@ -252,11 +253,8 @@ class ActionsMapParser(BaseActionsMapParser):
return [name, full] return [name, full]
return [name] return [name]
def add_global_parser(self, **kwargs): def has_global_parser(self):
if not self._global_parser: return True
self._global_parser = self._parser.add_argument_group(
"global arguments")
return self._global_parser
def add_category_parser(self, name, category_help=None, **kwargs): def add_category_parser(self, name, category_help=None, **kwargs):
"""Add a parser for a category """Add a parser for a category
@ -290,6 +288,15 @@ class ActionsMapParser(BaseActionsMapParser):
deprecated=deprecated, deprecated=deprecated,
deprecated_alias=deprecated_alias) 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): def parse_args(self, args, **kwargs):
try: try:
ret = self._parser.parse_args(args) ret = self._parser.parse_args(args)