Add mechanism for storing warnings and deprecations outside of AnsibleModule (#58993)
* Move warn() and deprecate() methods out of basic.py * Use _global_warnings and _global_deprications and create accessor functions - This lays the foundation for future functions being moved outside of AnsibleModule that need an interface to warnings and deprecations without modifying them. * Add unit tests for new warn and deprecate functions
This commit is contained in:
parent
cc2376b782
commit
3461c682c3
7 changed files with 193 additions and 30 deletions
2
changelogs/fragments/add-global-warnings-container.yaml
Normal file
2
changelogs/fragments/add-global-warnings-container.yaml
Normal file
|
@ -0,0 +1,2 @@
|
|||
minor_changes:
|
||||
- add mechanism for storing warnings and deprecations globally and not attached to an ``AnsibleModule`` object (https://github.com/ansible/ansible/pull/58993)
|
|
@ -193,6 +193,12 @@ from ansible.module_utils.common.validation import (
|
|||
)
|
||||
from ansible.module_utils.common._utils import get_all_subclasses as _get_all_subclasses
|
||||
from ansible.module_utils.parsing.convert_bool import BOOLEANS, BOOLEANS_FALSE, BOOLEANS_TRUE, boolean
|
||||
from ansible.module_utils.common.warnings import (
|
||||
deprecate,
|
||||
get_deprecation_messages,
|
||||
get_warning_messages,
|
||||
warn,
|
||||
)
|
||||
|
||||
# Note: When getting Sequence from collections, it matches with strings. If
|
||||
# this matters, make sure to check for strings before checking for sequencetype
|
||||
|
@ -612,8 +618,6 @@ class AnsibleModule(object):
|
|||
# May be used to set modifications to the environment for any
|
||||
# run_command invocation
|
||||
self.run_command_environ_update = {}
|
||||
self._warnings = []
|
||||
self._deprecations = []
|
||||
self._clean = {}
|
||||
self._string_conversion_action = ''
|
||||
|
||||
|
@ -728,22 +732,12 @@ class AnsibleModule(object):
|
|||
return self._tmpdir
|
||||
|
||||
def warn(self, warning):
|
||||
|
||||
if isinstance(warning, string_types):
|
||||
self._warnings.append(warning)
|
||||
self.log('[WARNING] %s' % warning)
|
||||
else:
|
||||
raise TypeError("warn requires a string not a %s" % type(warning))
|
||||
warn(warning)
|
||||
self.log('[WARNING] %s' % warning)
|
||||
|
||||
def deprecate(self, msg, version=None):
|
||||
if isinstance(msg, string_types):
|
||||
self._deprecations.append({
|
||||
'msg': msg,
|
||||
'version': version
|
||||
})
|
||||
self.log('[DEPRECATION WARNING] %s %s' % (msg, version))
|
||||
else:
|
||||
raise TypeError("deprecate requires a string not a %s" % type(msg))
|
||||
deprecate(msg, version)
|
||||
self.log('[DEPRECATION WARNING] %s %s' % (msg, version))
|
||||
|
||||
def load_file_common_arguments(self, params):
|
||||
'''
|
||||
|
@ -1409,7 +1403,7 @@ class AnsibleModule(object):
|
|||
alias_warnings = []
|
||||
alias_results, self._legal_inputs = handle_aliases(spec, param, alias_warnings=alias_warnings)
|
||||
for option, alias in alias_warnings:
|
||||
self._warnings.append('Both option %s and its alias %s are set.' % (option_prefix + option, option_prefix + alias))
|
||||
warn('Both option %s and its alias %s are set.' % (option_prefix + option, option_prefix + alias))
|
||||
|
||||
deprecated_aliases = []
|
||||
for i in spec.keys():
|
||||
|
@ -1419,9 +1413,7 @@ class AnsibleModule(object):
|
|||
|
||||
for deprecation in deprecated_aliases:
|
||||
if deprecation['name'] in param.keys():
|
||||
self._deprecations.append(
|
||||
{'msg': "Alias '%s' is deprecated. See the module docs for more information" % deprecation['name'],
|
||||
'version': deprecation['version']})
|
||||
deprecate("Alias '%s' is deprecated. See the module docs for more information" % deprecation['name'], deprecation['version'])
|
||||
return alias_results
|
||||
|
||||
def _handle_no_log_values(self, spec=None, param=None):
|
||||
|
@ -1435,7 +1427,9 @@ class AnsibleModule(object):
|
|||
except TypeError as te:
|
||||
self.fail_json(msg="Failure when processing no_log parameters. Module invocation will be hidden. "
|
||||
"%s" % to_native(te), invocation={'module_args': 'HIDDEN DUE TO FAILURE'})
|
||||
self._deprecations.extend(list_deprecations(spec, param))
|
||||
|
||||
for msg, version in list_deprecations(spec, param):
|
||||
deprecate(msg, version)
|
||||
|
||||
def _check_arguments(self, spec=None, param=None, legal_inputs=None):
|
||||
self._syslog_facility = 'LOG_USER'
|
||||
|
@ -2026,8 +2020,9 @@ class AnsibleModule(object):
|
|||
else:
|
||||
self.warn(kwargs['warnings'])
|
||||
|
||||
if self._warnings:
|
||||
kwargs['warnings'] = self._warnings
|
||||
warnings = get_warning_messages()
|
||||
if warnings:
|
||||
kwargs['warnings'] = warnings
|
||||
|
||||
if 'deprecations' in kwargs:
|
||||
if isinstance(kwargs['deprecations'], list):
|
||||
|
@ -2041,8 +2036,9 @@ class AnsibleModule(object):
|
|||
else:
|
||||
self.deprecate(kwargs['deprecations']) # pylint: disable=ansible-deprecated-no-version
|
||||
|
||||
if self._deprecations:
|
||||
kwargs['deprecations'] = self._deprecations
|
||||
deprecations = get_deprecation_messages()
|
||||
if deprecations:
|
||||
kwargs['deprecations'] = deprecations
|
||||
|
||||
kwargs = remove_values(kwargs, self.no_log_values)
|
||||
print('\n%s' % self.jsonify(kwargs))
|
||||
|
|
35
lib/ansible/module_utils/common/warnings.py
Normal file
35
lib/ansible/module_utils/common/warnings.py
Normal file
|
@ -0,0 +1,35 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
# Copyright (c) 2019 Ansible Project
|
||||
# Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause)
|
||||
|
||||
from __future__ import absolute_import, division, print_function
|
||||
__metaclass__ = type
|
||||
|
||||
from ansible.module_utils.six import string_types
|
||||
|
||||
_global_warnings = []
|
||||
_global_deprecations = []
|
||||
|
||||
|
||||
def warn(warning):
|
||||
if isinstance(warning, string_types):
|
||||
_global_warnings.append(warning)
|
||||
else:
|
||||
raise TypeError("warn requires a string not a %s" % type(warning))
|
||||
|
||||
|
||||
def deprecate(msg, version=None):
|
||||
if isinstance(msg, string_types):
|
||||
_global_deprecations.append({'msg': msg, 'version': version})
|
||||
else:
|
||||
raise TypeError("deprecate requires a string not a %s" % type(msg))
|
||||
|
||||
|
||||
def get_warning_messages():
|
||||
"""Return a tuple of warning messages accumulated over this run"""
|
||||
return tuple(_global_warnings)
|
||||
|
||||
|
||||
def get_deprecation_messages():
|
||||
"""Return a tuple of deprecations accumulated over this run"""
|
||||
return tuple(_global_deprecations)
|
|
@ -48,6 +48,7 @@ MODULE_UTILS_BASIC_IMPORTS = frozenset((('ansible', '__init__'),
|
|||
('ansible', 'module_utils', 'common', 'parameters'),
|
||||
('ansible', 'module_utils', 'common', 'process'),
|
||||
('ansible', 'module_utils', 'common', 'sys_info'),
|
||||
('ansible', 'module_utils', 'common', 'warnings'),
|
||||
('ansible', 'module_utils', 'common', 'text', '__init__'),
|
||||
('ansible', 'module_utils', 'common', 'text', 'converters'),
|
||||
('ansible', 'module_utils', 'common', 'text', 'formatters'),
|
||||
|
@ -69,6 +70,7 @@ MODULE_UTILS_BASIC_FILES = frozenset(('ansible/module_utils/_text.py',
|
|||
'ansible/module_utils/common/_json_compat.py',
|
||||
'ansible/module_utils/common/collections.py',
|
||||
'ansible/module_utils/common/parameters.py',
|
||||
'ansible/module_utils/common/warnings.py',
|
||||
'ansible/module_utils/parsing/convert_bool.py',
|
||||
'ansible/module_utils/common/__init__.py',
|
||||
'ansible/module_utils/common/file.py',
|
||||
|
|
|
@ -14,6 +14,7 @@ import pytest
|
|||
|
||||
from units.compat.mock import MagicMock
|
||||
from ansible.module_utils import basic
|
||||
from ansible.module_utils.common.warnings import get_deprecation_messages, get_warning_messages
|
||||
from ansible.module_utils.six import integer_types
|
||||
from ansible.module_utils.six.moves import builtins
|
||||
|
||||
|
@ -238,7 +239,7 @@ class TestComplexArgSpecs:
|
|||
"""Test that the complex argspec issues a warning if we specify an option both with its canonical name and its alias"""
|
||||
am = basic.AnsibleModule(**complex_argspec)
|
||||
assert isinstance(am.params['foo'], str)
|
||||
assert 'Both option foo and its alias dup are set.' in am._warnings
|
||||
assert 'Both option foo and its alias dup are set.' in get_warning_messages()
|
||||
assert am.params['foo'] == 'hello2'
|
||||
|
||||
@pytest.mark.parametrize('stdin', [{'foo': 'hello', 'bam': 'test'}], indirect=['stdin'])
|
||||
|
@ -338,8 +339,8 @@ class TestComplexArgSpecs:
|
|||
"""Test a deprecated alias"""
|
||||
am = basic.AnsibleModule(**complex_argspec)
|
||||
|
||||
assert "Alias 'zodraz' is deprecated." in am._deprecations[0]['msg']
|
||||
assert am._deprecations[0]['version'] == '9.99'
|
||||
assert "Alias 'zodraz' is deprecated." in get_deprecation_messages()[0]['msg']
|
||||
assert get_deprecation_messages()[0]['version'] == '9.99'
|
||||
|
||||
|
||||
class TestComplexOptions:
|
||||
|
@ -615,7 +616,7 @@ def test_no_log_false(stdin, capfd):
|
|||
"arg_pass": {"no_log": False}
|
||||
}
|
||||
am = basic.AnsibleModule(arg_spec)
|
||||
assert "testing" not in am.no_log_values and not am._warnings
|
||||
assert "testing" not in am.no_log_values and not get_warning_messages()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("stdin", [{"arg_pass": "testing"}], indirect=["stdin"])
|
||||
|
@ -629,4 +630,4 @@ def test_no_log_none(stdin, capfd):
|
|||
# Omitting no_log is only picked up by _log_invocation, so the value never
|
||||
# makes it into am.no_log_values. Instead we can check for the warning
|
||||
# emitted by am._log_invocation.
|
||||
assert len(am._warnings) > 0
|
||||
assert len(get_warning_messages()) > 0
|
||||
|
|
66
test/units/module_utils/common/warnings/test_deprecate.py
Normal file
66
test/units/module_utils/common/warnings/test_deprecate.py
Normal file
|
@ -0,0 +1,66 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
# (c) 2019 Ansible Project
|
||||
# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
|
||||
|
||||
from __future__ import absolute_import, division, print_function
|
||||
__metaclass__ = type
|
||||
|
||||
import pytest
|
||||
|
||||
import ansible.module_utils.common.warnings as warnings
|
||||
|
||||
from ansible.module_utils.common.warnings import deprecate, get_deprecation_messages
|
||||
from ansible.module_utils.six import PY3
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def deprecation_messages():
|
||||
return [
|
||||
{'msg': 'First deprecation', 'version': None},
|
||||
{'msg': 'Second deprecation', 'version': '2.14'},
|
||||
{'msg': 'Third deprecation', 'version': '2.9'},
|
||||
]
|
||||
|
||||
|
||||
def test_deprecate_message_only():
|
||||
deprecate('Deprecation message')
|
||||
assert warnings._global_deprecations == [{'msg': 'Deprecation message', 'version': None}]
|
||||
|
||||
|
||||
def test_deprecate_with_version():
|
||||
deprecate(msg='Deprecation message', version='2.14')
|
||||
assert warnings._global_deprecations == [{'msg': 'Deprecation message', 'version': '2.14'}]
|
||||
|
||||
|
||||
def test_multiple_deprecations(deprecation_messages):
|
||||
for d in deprecation_messages:
|
||||
deprecate(**d)
|
||||
|
||||
assert deprecation_messages == warnings._global_deprecations
|
||||
|
||||
|
||||
def test_get_deprecation_messages(deprecation_messages):
|
||||
for d in deprecation_messages:
|
||||
deprecate(**d)
|
||||
|
||||
accessor_deprecations = get_deprecation_messages()
|
||||
assert isinstance(accessor_deprecations, tuple)
|
||||
assert len(accessor_deprecations) == 3
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
'test_case',
|
||||
(
|
||||
1,
|
||||
True,
|
||||
[1],
|
||||
{'k1': 'v1'},
|
||||
(1, 2),
|
||||
6.62607004,
|
||||
b'bytestr' if PY3 else None,
|
||||
None,
|
||||
)
|
||||
)
|
||||
def test_deprecate_failure(test_case):
|
||||
with pytest.raises(TypeError, match='deprecate requires a string not a %s' % type(test_case)):
|
||||
deprecate(test_case)
|
61
test/units/module_utils/common/warnings/test_warn.py
Normal file
61
test/units/module_utils/common/warnings/test_warn.py
Normal file
|
@ -0,0 +1,61 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
# (c) 2019 Ansible Project
|
||||
# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
|
||||
|
||||
from __future__ import absolute_import, division, print_function
|
||||
__metaclass__ = type
|
||||
|
||||
import pytest
|
||||
|
||||
import ansible.module_utils.common.warnings as warnings
|
||||
|
||||
from ansible.module_utils.common.warnings import warn, get_warning_messages
|
||||
from ansible.module_utils.six import PY3
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def warning_messages():
|
||||
return [
|
||||
'First warning',
|
||||
'Second warning',
|
||||
'Third warning',
|
||||
]
|
||||
|
||||
|
||||
def test_warn():
|
||||
warn('Warning message')
|
||||
assert warnings._global_warnings == ['Warning message']
|
||||
|
||||
|
||||
def test_multiple_warningss(warning_messages):
|
||||
for w in warning_messages:
|
||||
warn(w)
|
||||
|
||||
assert warning_messages == warnings._global_warnings
|
||||
|
||||
|
||||
def test_get_warning_messages(warning_messages):
|
||||
for w in warning_messages:
|
||||
warn(w)
|
||||
|
||||
accessor_warnings = get_warning_messages()
|
||||
assert isinstance(accessor_warnings, tuple)
|
||||
assert len(accessor_warnings) == 3
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
'test_case',
|
||||
(
|
||||
1,
|
||||
True,
|
||||
[1],
|
||||
{'k1': 'v1'},
|
||||
(1, 2),
|
||||
6.62607004,
|
||||
b'bytestr' if PY3 else None,
|
||||
None,
|
||||
)
|
||||
)
|
||||
def test_warn_failure(test_case):
|
||||
with pytest.raises(TypeError, match='warn requires a string not a %s' % type(test_case)):
|
||||
warn(test_case)
|
Loading…
Reference in a new issue