Skip to content

Commit

Permalink
Multiple modules using ModuleHelper (#4674)
Browse files Browse the repository at this point in the history
* Multiple modules using ModuleHelper

Replaced raising exception with calling method do_raise() in MH.
Removed the importing of the exception class.

* added changelog fragment
  • Loading branch information
russoz authored May 23, 2022
1 parent 319c29c commit 6052776
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 35 deletions.
6 changes: 6 additions & 0 deletions changelogs/fragments/4674-use-mh-raise.yaml
Original file line number Diff line number Diff line change
@@ -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).
17 changes: 8 additions & 9 deletions plugins/modules/packaging/language/cpanm.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@
import os

from ansible_collections.community.general.plugins.module_utils.module_helper import (
ModuleHelper, CmdMixin, ArgFormat, ModuleHelperException
ModuleHelper, CmdMixin, ArgFormat
)


Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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


Expand Down
16 changes: 6 additions & 10 deletions plugins/modules/packaging/language/pipx.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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),
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down
22 changes: 11 additions & 11 deletions plugins/modules/packaging/os/snap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)


Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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(',')

Expand Down Expand Up @@ -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):

Expand Down Expand Up @@ -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")
Expand All @@ -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
Expand All @@ -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'])
Expand Down
6 changes: 3 additions & 3 deletions plugins/modules/system/mksysb.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
import os

from ansible_collections.community.general.plugins.module_utils.module_helper import (
CmdModuleHelper, ArgFormat, ModuleHelperException
CmdModuleHelper, ArgFormat
)


Expand Down Expand Up @@ -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:
Expand All @@ -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


Expand Down
4 changes: 2 additions & 2 deletions plugins/modules/system/xfconf.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@
'''

from ansible_collections.community.general.plugins.module_utils.module_helper import (
CmdStateModuleHelper, ArgFormat, ModuleHelperException
CmdStateModuleHelper, ArgFormat
)


Expand Down Expand Up @@ -216,7 +216,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:
Expand Down

0 comments on commit 6052776

Please sign in to comment.