Multiple modules using ModuleHelper (#4674) (#4713)

* 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 6052776de1)

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

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).

plugins/modules/packaging/language/cpanm.py

@@ -134,7 +134,7 @@ EXAMPLES = '''
 import os
 
 from ansible_collections.community.general.plugins.module_utils.module_helper import (
-    ModuleHelper, CmdMixin, ArgFormat, ModuleHelperException
+    ModuleHelper, CmdMixin, ArgFormat
 )
 
 
@@ -171,10 +171,10 @@ class CPANMinus(CmdMixin, ModuleHelper):
         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 @@ class CPANMinus(CmdMixin, ModuleHelper):
 
         return rc == 0
 
-    @staticmethod
+    def sanitize_pkg_spec_version(self, pkg_spec, version):
-    def sanitize_pkg_spec_version(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 @@ class CPANMinus(CmdMixin, ModuleHelper):
 
     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
 
 

plugins/modules/packaging/language/pipx.py

@@ -134,7 +134,7 @@ EXAMPLES = '''
 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,8 +153,7 @@ class PipX(CmdStateModuleHelper):
     module = dict(
         argument_spec=dict(
             state=dict(type='str', default='install',
-                       choices=[
+                       choices=['present', 'absent', 'install', 'uninstall', 'uninstall_all',
-                           'present', 'absent', 'install', 'uninstall', 'uninstall_all',
                                 'inject', 'upgrade', 'upgrade_all', 'reinstall', 'reinstall_all']),
             name=dict(type='str'),
             source=dict(type='str'),
@@ -247,8 +246,7 @@ class PipX(CmdStateModuleHelper):
 
     def state_upgrade(self):
         if not self.vars.application:
-            raise ModuleHelperException(
+            self.do_raise("Trying to upgrade a non-existent application: {0}".format(self.vars.name))
-                "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 @@ class PipX(CmdStateModuleHelper):
 
     def state_reinstall(self):
         if not self.vars.application:
-            raise ModuleHelperException(
+            self.do_raise("Trying to reinstall a non-existent application: {0}".format(self.vars.name))
-                "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(
+            self.do_raise("Trying to inject packages into a non-existent application: {0}".format(self.vars.name))
-                "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:

plugins/modules/packaging/os/snap.py

@@ -147,7 +147,7 @@ import numbers
 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,7 +218,7 @@ class Snap(CmdStateModuleHelper):
         option_map = {}
 
         if not isinstance(json_subtree, dict):
-            raise ModuleHelperException("Non-dict non-leaf element encountered while parsing option map. "
+            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():
@@ -252,7 +252,7 @@ class Snap(CmdStateModuleHelper):
             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 @@ class Snap(CmdStateModuleHelper):
         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 @@ class Snap(CmdStateModuleHelper):
         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 @@ class Snap(CmdStateModuleHelper):
 
                 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 @@ class Snap(CmdStateModuleHelper):
                     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 @@ class Snap(CmdStateModuleHelper):
             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'])

plugins/modules/system/mksysb.py

@@ -99,7 +99,7 @@ msg:
 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 @@ class MkSysB(CmdModuleHelper):
 
     def process_command_output(self, rc, out, err):
         if rc != 0:
-            raise ModuleHelperException("mksysb failed.")
+            self.do_raise("mksysb failed.")
         self.vars.msg = out
 
 

plugins/modules/system/xfconf.py

@@ -146,7 +146,7 @@ RETURN = '''
 '''
 
 from ansible_collections.community.general.plugins.module_utils.module_helper import (
-    CmdStateModuleHelper, ArgFormat, ModuleHelperException
+    CmdStateModuleHelper, ArgFormat
 )
 
 
@@ -212,7 +212,7 @@ class XFConfProperty(CmdStateModuleHelper):
         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: