From 585dd0b6ed4f0fff0a99aef7ebd10da358f783c8 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky <103110+russoz@users.noreply.github.com> Date: Mon, 1 Mar 2021 03:06:36 +1300 Subject: [PATCH] Improved ModuleHelper.run_command() (#1867) * Improved run_command signature and behaviour - extra_params has been removed from the signature - params now can be either str or dict (containing the param value) * Reverted the removal of the method parameter, and added changelog fragment * Update changelogs/fragments/1867-modhelper-cmdmixin-dict-params.yml Co-authored-by: Felix Fontein * Update plugins/module_utils/module_helper.py Co-authored-by: Felix Fontein * adjustement per PR * Update plugins/module_utils/module_helper.py Co-authored-by: Felix Fontein Co-authored-by: Felix Fontein --- .../1867-modhelper-cmdmixin-dict-params.yml | 2 ++ plugins/module_utils/module_helper.py | 29 +++++++++++++------ plugins/modules/system/xfconf.py | 14 ++++----- 3 files changed, 27 insertions(+), 18 deletions(-) create mode 100644 changelogs/fragments/1867-modhelper-cmdmixin-dict-params.yml diff --git a/changelogs/fragments/1867-modhelper-cmdmixin-dict-params.yml b/changelogs/fragments/1867-modhelper-cmdmixin-dict-params.yml new file mode 100644 index 0000000000..3f757b233a --- /dev/null +++ b/changelogs/fragments/1867-modhelper-cmdmixin-dict-params.yml @@ -0,0 +1,2 @@ +minor_changes: + - module_helper module utils - ``CmdMixin.run_command()`` now accepts ``dict`` command arguments, providing the parameter and its value (https://github.com/ansible-collections/community.general/pull/1867). diff --git a/plugins/module_utils/module_helper.py b/plugins/module_utils/module_helper.py index f35db8283d..3d145e713e 100644 --- a/plugins/module_utils/module_helper.py +++ b/plugins/module_utils/module_helper.py @@ -296,17 +296,28 @@ class CmdMixin(object): param_list = params if params else self.module.params.keys() for param in param_list: - if param in self.module.argument_spec: - if param not in self.module.params: + if isinstance(param, dict): + if len(param) != 1: + raise ModuleHelperException("run_command parameter as a dict must " + "contain only one key: {0}".format(param)) + _param = list(param.keys())[0] + fmt = find_format(_param) + value = param[_param] + elif isinstance(param, str): + if param in self.module.argument_spec: + fmt = find_format(param) + value = self.module.params[param] + elif param in extra_params: + fmt = find_format(param) + value = extra_params[param] + else: + self.module.deprecate("Cannot determine value for parameter: {0}. " + "From version 4.0.0 onwards this will generate an exception".format(param), + version="4.0.0", collection_name="community.general") continue - fmt = find_format(param) - value = self.module.params[param] + else: - if param not in extra_params: - continue - fmt = find_format(param) - value = extra_params[param] - self.cmd_args = cmd_args + raise ModuleHelperException("run_command parameter must be either a str or a dict: {0}".format(param)) cmd_args = add_arg_formatted_param(cmd_args, fmt, value) return cmd_args diff --git a/plugins/modules/system/xfconf.py b/plugins/modules/system/xfconf.py index ce85a2ba47..b6e6110e87 100644 --- a/plugins/modules/system/xfconf.py +++ b/plugins/modules/system/xfconf.py @@ -238,7 +238,7 @@ class XFConfProperty(CmdMixin, StateMixin, ModuleHelper): def state_absent(self): self.vars.value = None - self.run_command(params=('channel', 'property', 'reset'), extra_params={"reset": True}) + self.run_command(params=('channel', 'property', {'reset': True})) self.update_xfconf_output(previous_value=self.vars.previous_value, value=None) @@ -267,17 +267,13 @@ class XFConfProperty(CmdMixin, StateMixin, ModuleHelper): isinstance(self.vars.previous_value, list) or \ values_len > 1 - params = ['channel', 'property', 'create'] + params = ['channel', 'property', {'create': True}] if self.vars.is_array: - params.append('is_array') - params.append('values_and_types') - - extra_params = dict(values_and_types=(self.vars.value, value_type)) - extra_params['create'] = True - extra_params['is_array'] = self.vars.is_array + params.append({'is_array': True}) + params.append({'values_and_types': (self.vars.value, value_type)}) if not self.module.check_mode: - self.run_command(params=params, extra_params=extra_params) + self.run_command(params=params) if not self.vars.is_array: self.vars.value = self.vars.value[0]