Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple modules using ModuleHelper #4674

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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:
Expand Down