From 58defa5cce4ac1095e35a21b167621c27649a34f Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Tue, 28 Oct 2014 00:15:02 -0500 Subject: [PATCH] Adding role deps to v2 Role class and fixing some bugs --- v2/ansible/parsing/yaml/__init__.py | 2 +- v2/ansible/parsing/yaml/objects.py | 2 +- v2/ansible/parsing/yaml/strings.py | 4 ---- v2/ansible/playbook/role.py | 29 +++++++++++++++++++---------- v2/test/playbook/test_role.py | 24 ++++++++++++++++++------ 5 files changed, 39 insertions(+), 22 deletions(-) diff --git a/v2/ansible/parsing/yaml/__init__.py b/v2/ansible/parsing/yaml/__init__.py index 6d121d991ec..0acb77f8fd3 100644 --- a/v2/ansible/parsing/yaml/__init__.py +++ b/v2/ansible/parsing/yaml/__init__.py @@ -80,7 +80,7 @@ class DataLoader(): # if the file has already been read in and cached, we'll # return those results to avoid more file/vault operations if file_name in self._FILE_CACHE: - return self._FILE_CACHE + return self._FILE_CACHE[file_name] # read the file contents and load the data structure from them (file_data, show_content) = self._get_file_contents(file_name) diff --git a/v2/ansible/parsing/yaml/objects.py b/v2/ansible/parsing/yaml/objects.py index ba89accd73a..6a7482fe497 100644 --- a/v2/ansible/parsing/yaml/objects.py +++ b/v2/ansible/parsing/yaml/objects.py @@ -37,7 +37,7 @@ class AnsibleBaseYAMLObject: self._line_number = line self._column_number = col - def copy_position_info(obj): + def copy_position_info(self, obj): ''' copies the position info from another object ''' assert isinstance(obj, AnsibleBaseYAMLObject) diff --git a/v2/ansible/parsing/yaml/strings.py b/v2/ansible/parsing/yaml/strings.py index b7e304194fc..a778904e633 100644 --- a/v2/ansible/parsing/yaml/strings.py +++ b/v2/ansible/parsing/yaml/strings.py @@ -15,10 +15,6 @@ # You should have received a copy of the GNU General Public License # along with Ansible. If not, see . -# Make coding more python3-ish -from __future__ import (absolute_import, division, print_function) -__metaclass__ = type - __all__ = [ 'YAML_SYNTAX_ERROR', 'YAML_POSITION_DETAILS', diff --git a/v2/ansible/playbook/role.py b/v2/ansible/playbook/role.py index 2d492ee5061..38a8ac195d0 100644 --- a/v2/ansible/playbook/role.py +++ b/v2/ansible/playbook/role.py @@ -93,11 +93,14 @@ class Role(Base): if cache_key in _ROLE_CACHE: r = _ROLE_CACHE[cache_key] else: - # load the role - r = Role() - r.load_data(data) - # and cache it for next time - _ROLE_CACHE[cache_key] = r + try: + # load the role + r = Role() + r.load_data(data) + # and cache it for next time + _ROLE_CACHE[cache_key] = r + except RuntimeError: + raise AnsibleError("A recursive loop was detected while loading your roles", obj=data) # now add the parent to the (new) role if parent_role: @@ -192,17 +195,13 @@ class Role(Base): # FIXME: this should use unfrackpath once the utils code has been sorted out role_path = os.path.normpath(role) - print("first role path is %s" % role_path) if os.path.exists(role_path): role_name = os.path.basename(role) - print('returning role path %s' % role_path) return (role_name, role_path) else: for path in ('./roles', '/etc/ansible/roles'): role_path = os.path.join(path, role) - print("current role path is %s" % role_path) if os.path.exists(role_path): - print('returning role path %s' % role_path) return (role, role_path) # FIXME: make the parser smart about list/string entries @@ -350,12 +349,22 @@ class Role(Base): return [] return self._load_list_of_blocks(ds) + def _load_dependencies(self, attr, ds): + assert type(ds) in (list, type(None)) + + deps = [] + if ds: + for role_def in ds: + r = Role.load(role_def, parent_role=self) + deps.append(r) + return deps + #------------------------------------------------------------------------------ # other functions def add_parent(self, parent_role): ''' adds a role to the list of this roles parents ''' - assert isinstance(role, Role) + assert isinstance(parent_role, Role) if parent_role not in self._parents: self._parents.append(parent_role) diff --git a/v2/test/playbook/test_role.py b/v2/test/playbook/test_role.py index d3138fa5764..094c5c3f494 100644 --- a/v2/test/playbook/test_role.py +++ b/v2/test/playbook/test_role.py @@ -119,10 +119,23 @@ class TestRole(unittest.TestCase): @patch.object(Role, '_load_role_yaml') def test_load_role_with_metadata(self, _load_role_yaml, _get_role_path): + def fake_get_role_path(role): + if role == 'foo': + return ('foo', '/etc/ansible/roles/foo') + elif role == 'bar': + return ('bar', '/etc/ansible/roles/bar') + elif role == 'bad1': + return ('bad1', '/etc/ansible/roles/bad1') + elif role == 'bad2': + return ('bad2', '/etc/ansible/roles/bad2') + def fake_load_role_yaml(role_path, subdir): if role_path == '/etc/ansible/roles/foo': if subdir == 'meta': return dict(dependencies=['bar'], allow_duplicates=True, galaxy_info=dict(a='1', b='2', c='3')) + elif role_path == '/etc/ansible/roles/bar': + if subdir == 'meta': + return dict() elif role_path == '/etc/ansible/roles/bad1': if subdir == 'meta': return 1 @@ -131,19 +144,18 @@ class TestRole(unittest.TestCase): return dict(foo='bar') return None + _get_role_path.side_effect = fake_get_role_path _load_role_yaml.side_effect = fake_load_role_yaml - _get_role_path.return_value = ('foo', '/etc/ansible/roles/foo') - r = Role.load('foo') - self.assertEqual(r.dependencies, ['bar']) + self.assertEqual(len(r.dependencies), 1) + self.assertEqual(type(r.dependencies[0]), Role) + self.assertEqual(len(r.dependencies[0]._parents), 1) + self.assertEqual(r.dependencies[0]._parents[0], r) self.assertEqual(r.allow_duplicates, True) self.assertEqual(r.galaxy_info, dict(a='1', b='2', c='3')) - _get_role_path.return_value = ('bad1', '/etc/ansible/roles/bad1') self.assertRaises(AnsibleParserError, Role.load, 'bad1') - - _get_role_path.return_value = ('bad2', '/etc/ansible/roles/bad2') self.assertRaises(AnsibleParserError, Role.load, 'bad2') @patch.object(Role, '_get_role_path')