From 9cdb97594d60ce4f4d8b964059298babd720adc9 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky <103110+russoz@users.noreply.github.com> Date: Mon, 23 May 2022 17:19:24 +1200 Subject: [PATCH] Multiple modules using ModuleHelper (#4674) * Multiple modules using ModuleHelper Replaced raising exception with calling method do_raise() in MH. Removed the importing of the exception class. * added changelog fragment (cherry picked from commit 6052776de138606fb23d7383476a321cb7ff2e00) --- changelogs/fragments/4674-use-mh-raise.yaml | 6 ++++++ plugins/modules/packaging/language/cpanm.py | 17 ++++++++-------- plugins/modules/packaging/language/pipx.py | 16 ++++++--------- plugins/modules/packaging/os/snap.py | 22 ++++++++++----------- plugins/modules/system/mksysb.py | 6 +++--- plugins/modules/system/xfconf.py | 4 ++-- 6 files changed, 36 insertions(+), 35 deletions(-) create mode 100644 changelogs/fragments/4674-use-mh-raise.yaml diff --git a/changelogs/fragments/4674-use-mh-raise.yaml b/changelogs/fragments/4674-use-mh-raise.yaml new file mode 100644 index 00000000000..3e8ad13975b --- /dev/null +++ b/changelogs/fragments/4674-use-mh-raise.yaml @@ -0,0 +1,6 @@ +minor_changes: + - cpanm - using ``do_raise()`` to raise exceptions in ``ModuleHelper`` derived modules (https://github.com/ansible-collections/community.general/pull/4674). + - pipx - using ``do_raise()`` to raise exceptions in ``ModuleHelper`` derived modules (https://github.com/ansible-collections/community.general/pull/4674). + - snap - using ``do_raise()`` to raise exceptions in ``ModuleHelper`` derived modules (https://github.com/ansible-collections/community.general/pull/4674). + - mksysb - using ``do_raise()`` to raise exceptions in ``ModuleHelper`` derived modules (https://github.com/ansible-collections/community.general/pull/4674). + - xfconf - using ``do_raise()`` to raise exceptions in ``ModuleHelper`` derived modules (https://github.com/ansible-collections/community.general/pull/4674). diff --git a/plugins/modules/packaging/language/cpanm.py b/plugins/modules/packaging/language/cpanm.py index 7b209d1faef..8c8f2ea1c39 100644 --- a/plugins/modules/packaging/language/cpanm.py +++ b/plugins/modules/packaging/language/cpanm.py @@ -134,7 +134,7 @@ import os from ansible_collections.community.general.plugins.module_utils.module_helper import ( - ModuleHelper, CmdMixin, ArgFormat, ModuleHelperException + ModuleHelper, CmdMixin, ArgFormat ) @@ -171,10 +171,10 @@ def __init_module__(self): v = self.vars if v.mode == "compatibility": if v.name_check: - raise ModuleHelperException("Parameter name_check can only be used with mode=new") + self.do_raise("Parameter name_check can only be used with mode=new") else: if v.name and v.from_path: - raise ModuleHelperException("Parameters 'name' and 'from_path' are mutually exclusive when 'mode=new'") + self.do_raise("Parameters 'name' and 'from_path' are mutually exclusive when 'mode=new'") self.command = self.module.get_bin_path(v.executable if v.executable else self.command) self.vars.set("binary", self.command) @@ -190,17 +190,16 @@ def _is_package_installed(self, name, locallib, version): return rc == 0 - @staticmethod - def sanitize_pkg_spec_version(pkg_spec, version): + def sanitize_pkg_spec_version(self, pkg_spec, version): if version is None: return pkg_spec if pkg_spec.endswith('.tar.gz'): - raise ModuleHelperException(msg="parameter 'version' must not be used when installing from a file") + self.do_raise(msg="parameter 'version' must not be used when installing from a file") if os.path.isdir(pkg_spec): - raise ModuleHelperException(msg="parameter 'version' must not be used when installing from a directory") + self.do_raise(msg="parameter 'version' must not be used when installing from a directory") if pkg_spec.endswith('.git'): if version.startswith('~'): - raise ModuleHelperException(msg="operator '~' not allowed in version parameter when installing from git repository") + self.do_raise(msg="operator '~' not allowed in version parameter when installing from git repository") version = version if version.startswith('@') else '@' + version elif version[0] not in ('@', '~'): version = '~' + version @@ -228,7 +227,7 @@ def __run__(self): def process_command_output(self, rc, out, err): if self.vars.mode == "compatibility" and rc != 0: - raise ModuleHelperException(msg=err, cmd=self.vars.cmd_args) + self.do_raise(msg=err, cmd=self.vars.cmd_args) return 'is up to date' not in err and 'is up to date' not in out diff --git a/plugins/modules/packaging/language/pipx.py b/plugins/modules/packaging/language/pipx.py index 0b2276c6f84..0d1103000a2 100644 --- a/plugins/modules/packaging/language/pipx.py +++ b/plugins/modules/packaging/language/pipx.py @@ -134,7 +134,7 @@ import json from ansible_collections.community.general.plugins.module_utils.module_helper import ( - CmdStateModuleHelper, ArgFormat, ModuleHelperException + CmdStateModuleHelper, ArgFormat ) from ansible.module_utils.facts.compat import ansible_facts @@ -153,9 +153,8 @@ class PipX(CmdStateModuleHelper): module = dict( argument_spec=dict( state=dict(type='str', default='install', - choices=[ - 'present', 'absent', 'install', 'uninstall', 'uninstall_all', - 'inject', 'upgrade', 'upgrade_all', 'reinstall', 'reinstall_all']), + choices=['present', 'absent', 'install', 'uninstall', 'uninstall_all', + 'inject', 'upgrade', 'upgrade_all', 'reinstall', 'reinstall_all']), name=dict(type='str'), source=dict(type='str'), install_deps=dict(type='bool', default=False), @@ -247,8 +246,7 @@ def state_install(self): def state_upgrade(self): if not self.vars.application: - raise ModuleHelperException( - "Trying to upgrade a non-existent application: {0}".format(self.vars.name)) + self.do_raise("Trying to upgrade a non-existent application: {0}".format(self.vars.name)) if self.vars.force: self.changed = True if not self.module.check_mode: @@ -262,16 +260,14 @@ def state_uninstall(self): def state_reinstall(self): if not self.vars.application: - raise ModuleHelperException( - "Trying to reinstall a non-existent application: {0}".format(self.vars.name)) + self.do_raise("Trying to reinstall a non-existent application: {0}".format(self.vars.name)) self.changed = True if not self.module.check_mode: self.run_command(params=['state', 'name', 'python']) def state_inject(self): if not self.vars.application: - raise ModuleHelperException( - "Trying to inject packages into a non-existent application: {0}".format(self.vars.name)) + self.do_raise("Trying to inject packages into a non-existent application: {0}".format(self.vars.name)) if self.vars.force: self.changed = True if not self.module.check_mode: diff --git a/plugins/modules/packaging/os/snap.py b/plugins/modules/packaging/os/snap.py index f5df9d1a52e..9ac56d09bd0 100644 --- a/plugins/modules/packaging/os/snap.py +++ b/plugins/modules/packaging/os/snap.py @@ -147,7 +147,7 @@ from ansible.module_utils.common.text.converters import to_native from ansible_collections.community.general.plugins.module_utils.module_helper import ( - CmdStateModuleHelper, ArgFormat, ModuleHelperException + CmdStateModuleHelper, ArgFormat ) @@ -218,8 +218,8 @@ def convert_json_subtree_to_map(self, json_subtree, prefix=None): option_map = {} if not isinstance(json_subtree, dict): - raise ModuleHelperException("Non-dict non-leaf element encountered while parsing option map. " - "The output format of 'snap set' may have changed. Aborting!") + self.do_raise("Non-dict non-leaf element encountered while parsing option map. " + "The output format of 'snap set' may have changed. Aborting!") for key, value in json_subtree.items(): full_key = key if prefix is None else prefix + "." + key @@ -252,7 +252,7 @@ def retrieve_option_map(self, snap_name): option_map = self.convert_json_to_map(out) except Exception as e: - raise ModuleHelperException( + self.do_raise( msg="Parsing option map returned by 'snap get {0}' triggers exception '{1}', output:\n'{2}'".format(snap_name, str(e), out)) return option_map @@ -267,7 +267,7 @@ def is_snap_enabled(self, snap_name): result = out.splitlines()[1] match = self.__disable_re.match(result) if not match: - raise ModuleHelperException(msg="Unable to parse 'snap list {0}' output:\n{1}".format(snap_name, out)) + self.do_raise(msg="Unable to parse 'snap list {0}' output:\n{1}".format(snap_name, out)) notes = match.group('notes') return "disabled" not in notes.split(',') @@ -300,7 +300,7 @@ def process_actionable_snaps(self, actionable_snaps): else: msg = "Ooops! Snap installation failed while executing '{cmd}', please examine logs and " \ "error output for more details.".format(cmd=self.vars.cmd) - raise ModuleHelperException(msg=msg) + self.do_raise(msg=msg) def state_present(self): @@ -330,14 +330,14 @@ def set_options(self): if not match: msg = "Cannot parse set option '{option_string}'".format(option_string=option_string) - raise ModuleHelperException(msg) + self.do_raise(msg) snap_prefix = match.group("snap_prefix") selected_snap_name = snap_prefix[:-1] if snap_prefix else None if selected_snap_name is not None and selected_snap_name not in self.vars.name: msg = "Snap option '{option_string}' refers to snap which is not in the list of snap names".format(option_string=option_string) - raise ModuleHelperException(msg) + self.do_raise(msg) if selected_snap_name is None or (snap_name is not None and snap_name == selected_snap_name): key = match.group("key") @@ -360,11 +360,11 @@ def set_options(self): if rc != 0: if 'has no "configure" hook' in err: msg = "Snap '{snap}' does not have any configurable options".format(snap=snap_name) - raise ModuleHelperException(msg) + self.do_raise(msg) msg = "Cannot set options '{options}' for snap '{snap}': error={error}".format( options=" ".join(options_changed), snap=snap_name, error=err) - raise ModuleHelperException(msg) + self.do_raise(msg) if overall_options_changed: self.vars.options_changed = overall_options_changed @@ -385,7 +385,7 @@ def _generic_state_action(self, actionable_func, actionable_var, params=None): return msg = "Ooops! Snap operation failed while executing '{cmd}', please examine logs and " \ "error output for more details.".format(cmd=self.vars.cmd) - raise ModuleHelperException(msg=msg) + self.do_raise(msg=msg) def state_absent(self): self._generic_state_action(self.is_snap_installed, "snaps_removed", ['classic', 'channel', 'state']) diff --git a/plugins/modules/system/mksysb.py b/plugins/modules/system/mksysb.py index ddf9c171f3b..8dd1b45cd0a 100644 --- a/plugins/modules/system/mksysb.py +++ b/plugins/modules/system/mksysb.py @@ -99,7 +99,7 @@ import os from ansible_collections.community.general.plugins.module_utils.module_helper import ( - CmdModuleHelper, ArgFormat, ModuleHelperException + CmdModuleHelper, ArgFormat ) @@ -136,7 +136,7 @@ class MkSysB(CmdModuleHelper): def __init_module__(self): if not os.path.isdir(self.vars.storage_path): - raise ModuleHelperException("Storage path %s is not valid." % self.vars.storage_path) + self.do_raise("Storage path %s is not valid." % self.vars.storage_path) def __run__(self): if not self.module.check_mode: @@ -149,7 +149,7 @@ def __run__(self): def process_command_output(self, rc, out, err): if rc != 0: - raise ModuleHelperException("mksysb failed.") + self.do_raise("mksysb failed.") self.vars.msg = out diff --git a/plugins/modules/system/xfconf.py b/plugins/modules/system/xfconf.py index 8becd4a6830..942f8f45d2c 100644 --- a/plugins/modules/system/xfconf.py +++ b/plugins/modules/system/xfconf.py @@ -146,7 +146,7 @@ ''' from ansible_collections.community.general.plugins.module_utils.module_helper import ( - CmdStateModuleHelper, ArgFormat, ModuleHelperException + CmdStateModuleHelper, ArgFormat ) @@ -212,7 +212,7 @@ def __init_module__(self): self.vars.meta('value').set(initial_value=self.vars.previous_value) if self.module.params['disable_facts'] is False: - raise ModuleHelperException('Returning results as facts has been removed. Stop using disable_facts=false.') + self.do_raise('Returning results as facts has been removed. Stop using disable_facts=false.') def process_command_output(self, rc, out, err): if err.rstrip() == self.does_not: