From d8ae4dfbf212000e238dbdd2ed9a99dd829bdc01 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Wed, 22 Nov 2017 14:35:58 -0600 Subject: [PATCH] Adding aliases for field attributes and renaming async attribute (#33141) * Adding aliases for field attributes and renaming async attribute As of Python 3.7, the use of async raises an error, whereas before the use of the reserved word was ignored. This adds an alias field for field attrs so that both async and async_val (interally) work. This allows us to be backwards-compatible with 3rd party plugins that may still reference Task.async, but for the core engine to work on Py3.7+. * Remove files fixed for 'async' usage from the python 3.7 skip list --- lib/ansible/executor/task_executor.py | 4 ++-- lib/ansible/playbook/attribute.py | 5 ++++- lib/ansible/playbook/base.py | 20 ++++++++++++++++--- lib/ansible/playbook/task.py | 2 +- lib/ansible/plugins/action/__init__.py | 8 ++++---- lib/ansible/plugins/action/net_base.py | 2 +- lib/ansible/plugins/action/normal.py | 2 +- lib/ansible/plugins/action/package.py | 2 +- lib/ansible/plugins/action/service.py | 2 +- test/compile/python3.7-skip.txt | 10 ---------- test/units/executor/test_task_executor.py | 6 +++--- test/units/plugins/action/test_action.py | 6 +++--- test/units/plugins/action/test_raw.py | 8 ++++---- test/units/plugins/action/test_synchronize.py | 2 +- 14 files changed, 43 insertions(+), 36 deletions(-) diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index c4a9213657..e883a64ddf 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -537,7 +537,7 @@ class TaskExecutor: if self._task.register: vars_copy[self._task.register] = wrap_var(result.copy()) - if self._task.async > 0: + if self._task.async_val > 0: if self._task.poll > 0 and not result.get('skipped') and not result.get('failed'): result = self._poll_async_result(result=result, templar=templar, task_vars=vars_copy) # FIXME callback 'v2_runner_on_async_poll' here @@ -668,7 +668,7 @@ class TaskExecutor: shared_loader_obj=self._shared_loader_obj, ) - time_left = self._task.async + time_left = self._task.async_val while time_left > 0: time.sleep(self._task.poll) diff --git a/lib/ansible/playbook/attribute.py b/lib/ansible/playbook/attribute.py index 5eab1753ff..455f7ded19 100644 --- a/lib/ansible/playbook/attribute.py +++ b/lib/ansible/playbook/attribute.py @@ -25,7 +25,7 @@ from copy import deepcopy class Attribute: def __init__(self, isa=None, private=False, default=None, required=False, listof=None, priority=0, class_type=None, always_post_validate=False, - inherit=True): + inherit=True, alias=None): """ :class:`Attribute` specifies constraints for attributes of objects which derive from playbook data. The attributes of the object are basically @@ -53,6 +53,8 @@ class Attribute: :kwarg inherit: A boolean value, which controls whether the object containing this field should attempt to inherit the value from its parent object if the local value is None. + :kwarg alias: An alias to use for the attribute name, for situations where + the attribute name may conflict with a Python reserved word. """ self.isa = isa @@ -64,6 +66,7 @@ class Attribute: self.class_type = class_type self.always_post_validate = always_post_validate self.inherit = inherit + self.alias = alias if default is not None and self.isa in ('list', 'dict', 'set'): self.default = deepcopy(default) diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py index 4a27eaea6d..4beb56f55c 100644 --- a/lib/ansible/playbook/base.py +++ b/lib/ansible/playbook/base.py @@ -109,6 +109,11 @@ class BaseMeta(type): dst_dict['_valid_attrs'][attr_name] = value dst_dict['_attributes'][attr_name] = value.default + if value.alias is not None: + dst_dict[value.alias] = property(getter, setter, deleter) + dst_dict['_valid_attrs'][value.alias] = value + dst_dict['_alias_attrs'][value.alias] = attr_name + def _process_parents(parents, dst_dict): ''' Helper method which creates attributes from all parent objects @@ -124,6 +129,7 @@ class BaseMeta(type): # create some additional class attributes dct['_attributes'] = dict() dct['_valid_attrs'] = dict() + dct['_alias_attrs'] = dict() # now create the attributes based on the FieldAttributes # available, including from parent (and grandparent) objects @@ -235,12 +241,15 @@ class Base(with_metaclass(BaseMeta, object)): # so that certain fields can be loaded before others, if they are dependent. for name, attr in sorted(iteritems(self._valid_attrs), key=operator.itemgetter(1)): # copy the value over unless a _load_field method is defined + target_name = name + if name in self._alias_attrs: + target_name = self._alias_attrs[name] if name in ds: method = getattr(self, '_load_%s' % name, None) if method: - self._attributes[name] = method(name, ds[name]) + self._attributes[target_name] = method(name, ds[name]) else: - self._attributes[name] = ds[name] + self._attributes[target_name] = ds[name] # run early, non-critical validation self.validate() @@ -279,13 +288,16 @@ class Base(with_metaclass(BaseMeta, object)): # walk all fields in the object for (name, attribute) in iteritems(self._valid_attrs): + if name in self._alias_attrs: + name = self._alias_attrs[name] + # run validator only if present method = getattr(self, '_validate_%s' % name, None) if method: method(attribute, name, getattr(self, name)) else: # and make sure the attribute is of the type it should be - value = getattr(self, name) + value = self._attributes[name] if value is not None: if attribute.isa == 'string' and isinstance(value, (list, dict)): raise AnsibleParserError( @@ -314,6 +326,8 @@ class Base(with_metaclass(BaseMeta, object)): new_me = self.__class__() for name in self._valid_attrs.keys(): + if name in self._alias_attrs: + continue new_me._attributes[name] = shallowcopy(self._attributes[name]) new_me._loader = self._loader diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 5e55cf7a1e..cbedc2b4fe 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -69,7 +69,7 @@ class Task(Base, Conditional, Taggable, Become): _args = FieldAttribute(isa='dict', default=dict()) _action = FieldAttribute(isa='string') - _async = FieldAttribute(isa='int', default=0) + _async_val = FieldAttribute(isa='int', default=0, alias='async') _changed_when = FieldAttribute(isa='list', default=[]) _delay = FieldAttribute(isa='int', default=5) _delegate_to = FieldAttribute(isa='string') diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index e7f9c95fed..ad6eb0bc25 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -93,11 +93,11 @@ class ActionBase(with_metaclass(ABCMeta, object)): result = {} - if self._task.async and not self._supports_async: + if self._task.async_val and not self._supports_async: raise AnsibleActionFail('async is not supported for this task.') elif self._play_context.check_mode and not self._supports_check_mode: raise AnsibleActionSkip('check mode is not supported for this task.') - elif self._task.async and self._play_context.check_mode: + elif self._task.async_val and self._play_context.check_mode: raise AnsibleActionFail('check mode and async cannot be used on same task.') return result @@ -159,7 +159,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): (module_data, module_style, module_shebang) = modify_module(module_name, module_path, module_args, task_vars=task_vars, module_compression=self._play_context.module_compression, - async_timeout=self._task.async, become=self._play_context.become, + async_timeout=self._task.async_val, become=self._play_context.become, become_method=self._play_context.become_method, become_user=self._play_context.become_user, become_password=self._play_context.become_pass, environment=final_environment) @@ -703,7 +703,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): self._transfer_data(remote_async_module_path, async_module_data) remote_files.append(remote_async_module_path) - async_limit = self._task.async + async_limit = self._task.async_val async_jid = str(random.randint(0, 999999999999)) # call the interpreter for async_wrapper directly diff --git a/lib/ansible/plugins/action/net_base.py b/lib/ansible/plugins/action/net_base.py index dc2e8864fd..437e0898a6 100644 --- a/lib/ansible/plugins/action/net_base.py +++ b/lib/ansible/plugins/action/net_base.py @@ -114,7 +114,7 @@ class ActionModule(ActionBase): display.vvvv('Running implementation module %s' % module) result.update(self._execute_module(module_name=module, module_args=new_module_args, task_vars=task_vars, - wrap_async=self._task.async)) + wrap_async=self._task.async_val)) display.vvvv('Caching network OS %s in facts' % play_context.network_os) result['ansible_facts'] = {'network_os': play_context.network_os} diff --git a/lib/ansible/plugins/action/normal.py b/lib/ansible/plugins/action/normal.py index 070942a9f4..f04888c25e 100644 --- a/lib/ansible/plugins/action/normal.py +++ b/lib/ansible/plugins/action/normal.py @@ -39,7 +39,7 @@ class ActionModule(ActionBase): del results['invocation']['module_args'] # FUTURE: better to let _execute_module calculate this internally? - wrap_async = self._task.async and not self._connection.has_native_async + wrap_async = self._task.async_val and not self._connection.has_native_async # do work! results = merge_hash(results, self._execute_module(tmp=tmp, task_vars=task_vars, wrap_async=wrap_async)) diff --git a/lib/ansible/plugins/action/package.py b/lib/ansible/plugins/action/package.py index a72880bf5b..f675f3fd8b 100644 --- a/lib/ansible/plugins/action/package.py +++ b/lib/ansible/plugins/action/package.py @@ -66,7 +66,7 @@ class ActionModule(ActionBase): del new_module_args['use'] display.vvvv("Running %s" % module) - result.update(self._execute_module(module_name=module, module_args=new_module_args, task_vars=task_vars, wrap_async=self._task.async)) + result.update(self._execute_module(module_name=module, module_args=new_module_args, task_vars=task_vars, wrap_async=self._task.async_val)) else: result['failed'] = True result['msg'] = 'Could not detect which package manager to use. Try gathering facts or setting the "use" option.' diff --git a/lib/ansible/plugins/action/service.py b/lib/ansible/plugins/action/service.py index a7c67fdee6..b3f3f5fd9d 100644 --- a/lib/ansible/plugins/action/service.py +++ b/lib/ansible/plugins/action/service.py @@ -74,7 +74,7 @@ class ActionModule(ActionBase): self._display.warning('Ignoring "%s" as it is not used in "%s"' % (unused, module)) self._display.vvvv("Running %s" % module) - result.update(self._execute_module(module_name=module, module_args=new_module_args, task_vars=task_vars, wrap_async=self._task.async)) + result.update(self._execute_module(module_name=module, module_args=new_module_args, task_vars=task_vars, wrap_async=self._task.async_val)) else: result['failed'] = True result['msg'] = 'Could not detect which service manager to use. Try gathering facts or setting the "use" option.' diff --git a/test/compile/python3.7-skip.txt b/test/compile/python3.7-skip.txt index 4f78158a04..65a915223c 100644 --- a/test/compile/python3.7-skip.txt +++ b/test/compile/python3.7-skip.txt @@ -1,13 +1,3 @@ lib/ansible/cli/adhoc.py -lib/ansible/executor/task_executor.py lib/ansible/modules/packaging/os/yum_repository.py -lib/ansible/plugins/action/__init__.py -lib/ansible/plugins/action/net_base.py -lib/ansible/plugins/action/normal.py -lib/ansible/plugins/action/package.py -lib/ansible/plugins/action/service.py -test/units/executor/test_task_executor.py test/units/modules/network/nuage/test_nuage_vspk.py -test/units/plugins/action/test_action.py -test/units/plugins/action/test_raw.py -test/units/plugins/action/test_synchronize.py diff --git a/test/units/executor/test_task_executor.py b/test/units/executor/test_task_executor.py index fc69f2dbc1..c540390b56 100644 --- a/test/units/executor/test_task_executor.py +++ b/test/units/executor/test_task_executor.py @@ -370,11 +370,11 @@ class TestTaskExecutor(unittest.TestCase): mock_task.changed_when = None mock_task.failed_when = None mock_task.post_validate.return_value = None - # mock_task.async cannot be left unset, because on Python 3 MagicMock() + # mock_task.async_val cannot be left unset, because on Python 3 MagicMock() # > 0 raises a TypeError There are two reasons for using the value 1 # here: on Python 2 comparing MagicMock() > 0 returns True, and the # other reason is that if I specify 0 here, the test fails. ;) - mock_task.async = 1 + mock_task.async_val = 1 mock_task.poll = 0 mock_play_context = MagicMock() @@ -431,7 +431,7 @@ class TestTaskExecutor(unittest.TestCase): mock_host = MagicMock() mock_task = MagicMock() - mock_task.async = 0.1 + mock_task.async_val = 0.1 mock_task.poll = 0.05 mock_play_context = MagicMock() diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index 9f460b065b..0139732b63 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -75,12 +75,12 @@ class TestActionBase(unittest.TestCase): play_context = PlayContext() - mock_task.async = None + mock_task.async_val = None action_base = DerivedActionBase(mock_task, mock_connection, play_context, None, None, None) results = action_base.run() self.assertEqual(results, dict()) - mock_task.async = 0 + mock_task.async_val = 0 action_base = DerivedActionBase(mock_task, mock_connection, play_context, None, None, None) results = action_base.run() self.assertEqual(results, {}) @@ -92,7 +92,7 @@ class TestActionBase(unittest.TestCase): # create our fake task mock_task = MagicMock() mock_task.action = "copy" - mock_task.async = 0 + mock_task.async_val = 0 # create a mock connection, so we don't actually try and connect to things mock_connection = MagicMock() diff --git a/test/units/plugins/action/test_raw.py b/test/units/plugins/action/test_raw.py index e0a2eb97a5..f082025d6b 100644 --- a/test/units/plugins/action/test_raw.py +++ b/test/units/plugins/action/test_raw.py @@ -43,7 +43,7 @@ class TestCopyResultExclude(unittest.TestCase): play_context = Mock() task = MagicMock(Task) - task.async = False + task.async_val = False connection = Mock() task.args = {'_raw_params': 'Args1'} @@ -60,7 +60,7 @@ class TestCopyResultExclude(unittest.TestCase): play_context = Mock() task = MagicMock(Task) - task.async = False + task.async_val = False connection = Mock() task.args = {'_raw_params': 'Args1'} @@ -75,7 +75,7 @@ class TestCopyResultExclude(unittest.TestCase): play_context = Mock() task = MagicMock(Task) - task.async = False + task.async_val = False connection = Mock() task.args = {'_raw_params': 'Args1'} @@ -92,7 +92,7 @@ class TestCopyResultExclude(unittest.TestCase): play_context = Mock() task = MagicMock(Task) - task.async = False + task.async_val = False connection = Mock() task.args = {'_raw_params': 'Args1'} diff --git a/test/units/plugins/action/test_synchronize.py b/test/units/plugins/action/test_synchronize.py index 53d73c500c..ad78f9ccaf 100644 --- a/test/units/plugins/action/test_synchronize.py +++ b/test/units/plugins/action/test_synchronize.py @@ -46,7 +46,7 @@ class TaskMock(object): args = {'src': u'/tmp/deleteme', 'dest': '/tmp/deleteme', 'rsync_path': 'rsync'} - async = None + async_val = None become = None become_user = None become_method = None