From 59843472713eaa1dc774d12aafa4892f427698d6 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Thu, 9 May 2019 15:40:55 -0400 Subject: [PATCH 01/17] Split building Reqs from cli strings to requirements_from_strings Get the list of Reqs in the cli, and pass into install_repository_specs_loops() 'editable' and 'namespace_override' are only needed for creating the Requirement() objects, so dont pass into loop --- ansible_galaxy/actions/install.py | 43 +++++++++++++++---------------- ansible_galaxy_cli/cli/galaxy.py | 9 ++++--- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/ansible_galaxy/actions/install.py b/ansible_galaxy/actions/install.py index e17701a9..cf957ae5 100644 --- a/ansible_galaxy/actions/install.py +++ b/ansible_galaxy/actions/install.py @@ -49,8 +49,6 @@ def _verify_requirements_repository_spec_have_namespaces(requirements_list): # pass a list of repository_spec objects def install_repositories_matching_repository_specs(galaxy_context, requirements_list, - editable=False, - namespace_override=None, display_callback=None, # TODO: error handling callback ? ignore_errors=False, @@ -58,7 +56,6 @@ def install_repositories_matching_repository_specs(galaxy_context, force_overwrite=False): '''Install a set of repositories specified by repository_specs if they are not already installed''' - # log.debug('editable: %s', editable) log.debug('requirements_list: %s', requirements_list) _verify_requirements_repository_spec_have_namespaces(requirements_list) @@ -95,20 +92,10 @@ def install_repositories_matching_repository_specs(galaxy_context, force_overwrite=force_overwrite) -# FIXME: probably pass the point where passing around all the data to methods makes sense -# so probably needs a stateful class here -def install_repository_specs_loop(galaxy_context, - repository_spec_strings=None, - requirements_list=None, - editable=False, - namespace_override=None, - display_callback=None, - # TODO: error handling callback ? - ignore_errors=False, - no_deps=False, - force_overwrite=False): - - requirements_list = requirements_list or [] +def requirements_from_strings(repository_spec_strings, + namespace_override=None, + editable=False): + requirements_list = [] for repository_spec_string in repository_spec_strings: fetch_method = \ @@ -130,9 +117,8 @@ def install_repository_specs_loop(galaxy_context, log.debug('repository_spec_string: %s', repository_spec_string) tmp_downloaded_path = download.fetch_url(repository_spec_string, - # Note: ignore_certs is meant for galaxy server, - # overloaded to apply for arbitrary http[s] downloads here - validate_certs=not galaxy_context.server['ignore_certs']) + # This is for random remote_urls, so always validate_certs + validate_certs=True) spec_data = collection_artifact.load_data_from_collection_artifact(tmp_downloaded_path) # pretend like this is a local_file install now @@ -152,6 +138,21 @@ def install_repository_specs_loop(galaxy_context, requirements_list.append(req) + return requirements_list + + +# FIXME: probably pass the point where passing around all the data to methods makes sense +# so probably needs a stateful class here +def install_repository_specs_loop(galaxy_context, + requirements, + display_callback=None, + # TODO: error handling callback ? + ignore_errors=False, + no_deps=False, + force_overwrite=False): + + requirements_list = requirements + log.debug('requirements_list: %s', requirements_list) while True: @@ -170,8 +171,6 @@ def install_repository_specs_loop(galaxy_context, just_installed_repositories = \ install_repositories_matching_repository_specs(galaxy_context, requirements_list, - editable=editable, - namespace_override=namespace_override, display_callback=display_callback, ignore_errors=ignore_errors, no_deps=no_deps, diff --git a/ansible_galaxy_cli/cli/galaxy.py b/ansible_galaxy_cli/cli/galaxy.py index 57fd3f41..0bd1b53e 100644 --- a/ansible_galaxy_cli/cli/galaxy.py +++ b/ansible_galaxy_cli/cli/galaxy.py @@ -275,11 +275,14 @@ def execute_install(self): galaxy_context = self._get_galaxy_context(self.options, self.config) requested_spec_strings = self.args + requirements = \ + install.requirements_from_strings(repository_spec_strings=requested_spec_strings, + editable=self.options.editable_install, + namespace_override=self.options.namespace) + # TODO: build requirement_specs from requested_collection_specs strings rc = install.install_repository_specs_loop(galaxy_context, - editable=self.options.editable_install, - repository_spec_strings=requested_spec_strings, - namespace_override=self.options.namespace, + requirements, display_callback=self.display, ignore_errors=self.options.ignore_errors, no_deps=self.options.no_deps, From 0b848f133ef3dc6e46a3bd05a3190784e5cb4dd5 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Thu, 9 May 2019 15:53:58 -0400 Subject: [PATCH 02/17] Rename Requirement() install method to install_requirements install_repositories_matching_repository_specs() was no inaccurate as well as still being too long, so change it. --- ansible_galaxy/actions/install.py | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/ansible_galaxy/actions/install.py b/ansible_galaxy/actions/install.py index cf957ae5..e419bd0f 100644 --- a/ansible_galaxy/actions/install.py +++ b/ansible_galaxy/actions/install.py @@ -47,13 +47,13 @@ def _verify_requirements_repository_spec_have_namespaces(requirements_list): # pass a list of repository_spec objects -def install_repositories_matching_repository_specs(galaxy_context, - requirements_list, - display_callback=None, - # TODO: error handling callback ? - ignore_errors=False, - no_deps=False, - force_overwrite=False): +def install_requirements(galaxy_context, + requirements_list, + display_callback=None, + # TODO: error handling callback ? + ignore_errors=False, + no_deps=False, + force_overwrite=False): '''Install a set of repositories specified by repository_specs if they are not already installed''' log.debug('requirements_list: %s', requirements_list) @@ -155,6 +155,10 @@ def install_repository_specs_loop(galaxy_context, log.debug('requirements_list: %s', requirements_list) + for req in requirements_list: + display_callback('Installing %s' % req.requirement_spec.label, level='info') + + # Loop until there are no unresolved deps or we break while True: if not requirements_list: break @@ -169,12 +173,12 @@ def install_repository_specs_loop(galaxy_context, display_callback(msg, level='info') just_installed_repositories = \ - install_repositories_matching_repository_specs(galaxy_context, - requirements_list, - display_callback=display_callback, - ignore_errors=ignore_errors, - no_deps=no_deps, - force_overwrite=force_overwrite) + install_requirements(galaxy_context, + requirements_list, + display_callback=display_callback, + ignore_errors=ignore_errors, + no_deps=no_deps, + force_overwrite=force_overwrite) display_callback('Installed:', level='info') From c323f887f768479343ec1e3da0fd7d060f13aa93 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Fri, 10 May 2019 11:37:35 -0400 Subject: [PATCH 03/17] Reorder actions.install methods in order of use --- ansible_galaxy/actions/install.py | 104 ++++++++++++++---------------- 1 file changed, 48 insertions(+), 56 deletions(-) diff --git a/ansible_galaxy/actions/install.py b/ansible_galaxy/actions/install.py index e419bd0f..9032de78 100644 --- a/ansible_galaxy/actions/install.py +++ b/ansible_galaxy/actions/install.py @@ -141,62 +141,7 @@ def requirements_from_strings(repository_spec_strings, return requirements_list -# FIXME: probably pass the point where passing around all the data to methods makes sense -# so probably needs a stateful class here -def install_repository_specs_loop(galaxy_context, - requirements, - display_callback=None, - # TODO: error handling callback ? - ignore_errors=False, - no_deps=False, - force_overwrite=False): - - requirements_list = requirements - - log.debug('requirements_list: %s', requirements_list) - - for req in requirements_list: - display_callback('Installing %s' % req.requirement_spec.label, level='info') - - # Loop until there are no unresolved deps or we break - while True: - if not requirements_list: - break - - display_callback('Installing:', level='info') - - for req in requirements_list: - if req.repository_spec: - msg = ' %s (required by %s)' % (req.requirement_spec.label, req.repository_spec) - else: - msg = ' %s' % req.requirement_spec.label - display_callback(msg, level='info') - - just_installed_repositories = \ - install_requirements(galaxy_context, - requirements_list, - display_callback=display_callback, - ignore_errors=ignore_errors, - no_deps=no_deps, - force_overwrite=force_overwrite) - - display_callback('Installed:', level='info') - - for just_installed_repo in just_installed_repositories: - display_callback(' %s to %s' % (just_installed_repo.repository_spec, - just_installed_repo.path), - level='info') - - # set the repository_specs to search for to whatever the install reported as being needed yet - # requirements_list = new_requirements_list - requirements_list = find_new_deps_from_installed(galaxy_context, - just_installed_repositories, - no_deps=no_deps) - - # FIXME: what results to return? - return 0 - - +# NOTE: this is equiv to add deps to a transaction def find_new_deps_from_installed(galaxy_context, installed_repos, no_deps=False): if no_deps: return [] @@ -253,6 +198,53 @@ def find_new_deps_from_installed(galaxy_context, installed_repos, no_deps=False) return unsolved_deps_reqs +# FIXME: probably pass the point where passing around all the data to methods makes sense +# so probably needs a stateful class here +def install_repository_specs_loop(galaxy_context, + requirements, + display_callback=None, + # TODO: error handling callback ? + ignore_errors=False, + no_deps=False, + force_overwrite=False): + + requirements_list = requirements + + log.debug('requirements_list: %s', requirements_list) + + for req in requirements_list: + display_callback('Installing %s' % req.requirement_spec.label, level='info') + + # Loop until there are no unresolved deps or we break + while True: + if not requirements_list: + break + + just_installed_repositories = \ + install_requirements(galaxy_context, + requirements_list, + display_callback=display_callback, + ignore_errors=ignore_errors, + no_deps=no_deps, + force_overwrite=force_overwrite) + + # set the repository_specs to search for to whatever the install reported as being needed yet + # requirements_list = new_requirements_list + requirements_list = find_new_deps_from_installed(galaxy_context, + just_installed_repositories, + no_deps=no_deps) + + for req in requirements_list: + if req.repository_spec: + msg = 'Installing requirement %s (required by %s)' % (req.requirement_spec.label, req.repository_spec.label) + else: + msg = 'Installing requirement %s' % req.requirement_spec.label + display_callback(msg, level='info') + + # FIXME: what results to return? + return 0 + + # TODO: split into resolve, find/get metadata, resolve deps, download, install transaction def install_repositories(galaxy_context, requirements_to_install, From f87afdbba5b239e92c200736b8f827fabb9a0b91 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Fri, 10 May 2019 11:38:09 -0400 Subject: [PATCH 04/17] Add some more (not working atm) actions.install unit tests --- .../actions/test_install_action.py | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/ansible_galaxy/actions/test_install_action.py b/tests/ansible_galaxy/actions/test_install_action.py index 009b382b..7d322197 100644 --- a/tests/ansible_galaxy/actions/test_install_action.py +++ b/tests/ansible_galaxy/actions/test_install_action.py @@ -17,6 +17,55 @@ def display_callback(msg, **kwargs): log.debug(msg) +def test_requirements_from_strings(): + res = install.requirements_from_strings(['alikins.some_collection', + 'testuser.another']) + + log.debug('res: %s', res) + + assert isinstance(res, list) + reqs = [req for req in res] + for req in reqs: + assert isinstance(req, Requirement) + + namespaces = [x.requirement_spec.namespace for x in res] + assert 'alikins' in namespaces + assert 'testuser' in namespaces + + +def test_install_requirements(galaxy_context, mocker): + mock_irdb = mocker.patch('ansible_galaxy.actions.install.installed_repository_db.InstalledRepositoryDatabase', + name='the_mock_irdb') + mock_irdb2 = mocker.patch('ansible_galaxy.installed_repository_db.InstalledRepositoryDatabase', + name='the_mock_irdb2') + + repo_spec = RepositorySpec(namespace='alikins', name='some_collection', + version='4.2.1') + repo = Repository(repository_spec=repo_spec, installed=True) + + mock_irdb.select.return_value = [repo] + mock_irdb.by_requirement_spec.return_value = [repo] + mock_irdb2.select.return_value = [repo] + mock_irdb2.by_requirement_spec.return_value = [repo] + + log.debug('mock_irdb %s', mock_irdb) + # return_value=iter(['bar', 'baz'])) + + requirements = install.requirements_from_strings(['alikins.some_collection', + 'testuser.another']) + + log.debug('requirements: %s', requirements) + + res = install.install_requirements(galaxy_context, + requirements, + display_callback=None, + ignore_errors=False, + no_deps=False, + force_overwrite=True) + + log.debug('res: %s', res) + + def test_install_repos_empty_requirements(galaxy_context): requirements_to_install = [] From 492e033051b3e17b971717e81df5eb4d97801636 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Mon, 13 May 2019 11:22:23 -0400 Subject: [PATCH 05/17] Create one irdb and pass through action.install methods. ie, one InstalledRepositoryDatabase() instead of a new instance in each helper method. The IRDB class is stateless, but using the same ref makes unit testing easier. Update unit tests for better coverage of action.install --- ansible_galaxy/actions/install.py | 21 +++++-- ansible_galaxy/repository.py | 1 + .../actions/test_install_action.py | 55 ++++++++++++++----- tests/ansible_galaxy/test_matchers.py | 19 ++++++- 4 files changed, 75 insertions(+), 21 deletions(-) diff --git a/ansible_galaxy/actions/install.py b/ansible_galaxy/actions/install.py index 9032de78..0eccbd6f 100644 --- a/ansible_galaxy/actions/install.py +++ b/ansible_galaxy/actions/install.py @@ -48,6 +48,7 @@ def _verify_requirements_repository_spec_have_namespaces(requirements_list): # pass a list of repository_spec objects def install_requirements(galaxy_context, + irdb, requirements_list, display_callback=None, # TODO: error handling callback ? @@ -62,11 +63,11 @@ def install_requirements(galaxy_context, # FIXME: mv mv this filtering to it's own method # match any of the content specs for stuff we want to install + # TODO: this is part of building the install transaction # ie, see if it is already installed requested_repository_specs = [x.requirement_spec for x in requirements_list] repository_spec_match_filter = matchers.MatchRepositorySpecNamespaceName(requested_repository_specs) - irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) already_installed_generator = irdb.select(repository_spec_match_filter=repository_spec_match_filter) # FIXME: if/when GalaxyContent and InstalledGalaxyContent are attr.ib based and frozen and hashable @@ -83,9 +84,11 @@ def install_requirements(galaxy_context, else: requirements_to_install = [y for y in requirements_list if y.requirement_spec not in already_installed_repository_spec_set] - log.debug('repository_specs_to_install: %s', pprint.pformat(requirements_to_install)) + log.debug('requirements_to_install: %s', pprint.pformat(requirements_to_install)) - return install_repositories(galaxy_context, requirements_to_install, + return install_repositories(galaxy_context, + irdb, + requirements_to_install, display_callback=display_callback, ignore_errors=ignore_errors, no_deps=no_deps, @@ -215,6 +218,9 @@ def install_repository_specs_loop(galaxy_context, for req in requirements_list: display_callback('Installing %s' % req.requirement_spec.label, level='info') + # TODO: rename installed_db? installed_collections_db? icdb? + irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) + # Loop until there are no unresolved deps or we break while True: if not requirements_list: @@ -222,6 +228,7 @@ def install_repository_specs_loop(galaxy_context, just_installed_repositories = \ install_requirements(galaxy_context, + irdb, requirements_list, display_callback=display_callback, ignore_errors=ignore_errors, @@ -247,6 +254,7 @@ def install_repository_specs_loop(galaxy_context, # TODO: split into resolve, find/get metadata, resolve deps, download, install transaction def install_repositories(galaxy_context, + irdb, requirements_to_install, display_callback=None, # TODO: error handling callback ? @@ -271,7 +279,9 @@ def install_repositories(galaxy_context, # TODO: if the default ordering of repository_specs isnt useful, may need to tweak it for requirement_to_install in sorted(requirements_to_install_uniq): log.debug('requirement_to_install: %s', requirement_to_install) + installed_repositories = install_repository(galaxy_context, + irdb, requirement_to_install, display_callback=display_callback, ignore_errors=ignore_errors, @@ -302,6 +312,7 @@ def install_repositories(galaxy_context, def install_repository(galaxy_context, + irdb, requirement_to_install, display_callback=None, # TODO: error handling callback ? @@ -329,13 +340,12 @@ def install_repository(galaxy_context, log.debug('About to find() requested requirement_spec_to_install: %s', requirement_spec_to_install) # potential_repository_spec is a repo spec for the install candidate we potentially found. - irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) log.debug('Checking to see if %s is already installed', requirement_spec_to_install) already_installed_iter = irdb.by_requirement_spec(requirement_spec_to_install) already_installed = sorted(list(already_installed_iter)) - log.debug('already_installed: %s', already_installed) + log.debug('req_spec: %s already_installed: %s', requirement_spec_to_install, already_installed) if already_installed: for already_installed_repository in already_installed: @@ -371,6 +381,7 @@ def install_repository(galaxy_context, log.warning('Unable to find metadata for %s: %s', requirement_spec_to_install.label, e) # FIXME: raise dep error exception? raise_without_ignore(ignore_errors, e) + # continue return None diff --git a/ansible_galaxy/repository.py b/ansible_galaxy/repository.py index 61473c16..781a3cd4 100644 --- a/ansible_galaxy/repository.py +++ b/ansible_galaxy/repository.py @@ -144,6 +144,7 @@ def load_from_dir(content_dir, namespace_path, namespace, name, installed=True): # TODO: make existence of a galaxy.yml and a MANIFEST.json mutual exclude and raise an exception for that case col_info = None + # MANIFEST.json is higher prec than galaxy.yml if galaxy_yml_data: col_info = galaxy_yml_data diff --git a/tests/ansible_galaxy/actions/test_install_action.py b/tests/ansible_galaxy/actions/test_install_action.py index 7d322197..ccf8beff 100644 --- a/tests/ansible_galaxy/actions/test_install_action.py +++ b/tests/ansible_galaxy/actions/test_install_action.py @@ -2,6 +2,7 @@ import mock from ansible_galaxy.actions import install +from ansible_galaxy import installed_repository_db from ansible_galaxy import exceptions from ansible_galaxy import repository_spec from ansible_galaxy import requirements @@ -18,8 +19,10 @@ def display_callback(msg, **kwargs): def test_requirements_from_strings(): + # TODO: tests for local file, remote url, etc res = install.requirements_from_strings(['alikins.some_collection', - 'testuser.another']) + 'testuser.another', + ]) log.debug('res: %s', res) @@ -34,42 +37,59 @@ def test_requirements_from_strings(): def test_install_requirements(galaxy_context, mocker): - mock_irdb = mocker.patch('ansible_galaxy.actions.install.installed_repository_db.InstalledRepositoryDatabase', - name='the_mock_irdb') - mock_irdb2 = mocker.patch('ansible_galaxy.installed_repository_db.InstalledRepositoryDatabase', - name='the_mock_irdb2') - repo_spec = RepositorySpec(namespace='alikins', name='some_collection', version='4.2.1') repo = Repository(repository_spec=repo_spec, installed=True) - mock_irdb.select.return_value = [repo] - mock_irdb.by_requirement_spec.return_value = [repo] - mock_irdb2.select.return_value = [repo] - mock_irdb2.by_requirement_spec.return_value = [repo] - - log.debug('mock_irdb %s', mock_irdb) - # return_value=iter(['bar', 'baz'])) + other_repo = Repository(repository_spec=RepositorySpec(namespace='xxxxxxxxxx', + name='mmmmmmm', + version='11.12.99'), + installed=True) requirements = install.requirements_from_strings(['alikins.some_collection', 'testuser.another']) log.debug('requirements: %s', requirements) + mock_irdb2 = mocker.MagicMock(name='the_mock_irdb2') + mock_irdb2.select.return_value = [repo, other_repo] + # mock_irdb2.by_requirement_spec.return_value = [repo] + # irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) + + another_repo_spec = RepositorySpec(namespace='testuser', name='another', + version='11.12.99') + expected_installs = [Repository(repository_spec=another_repo_spec)] + + mocker.patch('ansible_galaxy.actions.install.install_repositories', + return_value=expected_installs) + res = install.install_requirements(galaxy_context, + mock_irdb2, requirements, display_callback=None, ignore_errors=False, no_deps=False, - force_overwrite=True) + force_overwrite=False) log.debug('res: %s', res) + log.debug('mock_irdb2: %s', mock_irdb2) + log.debug('mock_irdb2.call_args_list: %s', mock_irdb2.call_args_list) + + assert isinstance(res, list) + res_repo = res[0] + assert isinstance(res_repo, Repository) + assert isinstance(res_repo.repository_spec, RepositorySpec) + assert res_repo.repository_spec.namespace == 'testuser' + assert res_repo.repository_spec.name == 'another' def test_install_repos_empty_requirements(galaxy_context): requirements_to_install = [] + irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) + ret = install.install_repositories(galaxy_context, + irdb, requirements_to_install=requirements_to_install, display_callback=display_callback) @@ -90,7 +110,10 @@ def test_install_repositories(galaxy_context, mocker): mocker.patch('ansible_galaxy.actions.install.install_repository', return_value=expected_repos) + irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) + ret = install.install_repositories(galaxy_context, + irdb, requirements_to_install=requirements_to_install, display_callback=display_callback) @@ -141,7 +164,11 @@ def test_install_repositories_no_deps_required(galaxy_context, mocker): mocker.patch('ansible_galaxy.actions.install.install_repository', return_value=[]) + # ? Mock this instead? maybe a fixture? + irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) + ret = install.install_repositories(galaxy_context, + irdb, requirements_to_install=repository_specs_to_install, display_callback=display_callback) diff --git a/tests/ansible_galaxy/test_matchers.py b/tests/ansible_galaxy/test_matchers.py index 9abc78a5..02bbea6a 100644 --- a/tests/ansible_galaxy/test_matchers.py +++ b/tests/ansible_galaxy/test_matchers.py @@ -4,10 +4,18 @@ from ansible_galaxy import matchers from ansible_galaxy.models.repository import Repository from ansible_galaxy.models.repository_spec import RepositorySpec +from ansible_galaxy.models.requirement_spec import RequirementSpec log = logging.getLogger(__name__) +def RS(namespace=None, name=None, version_spec=None): + rs = RequirementSpec(namespace=namespace, + name=name, + version_spec=version_spec) + return rs + + def CR(namespace=None, name=None): cs = RepositorySpec(namespace=namespace, name=name, @@ -58,8 +66,15 @@ def CR(namespace=None, name=None): (matchers.MatchNamespacesOrLabels, (['ns2.name2'],), [CR(namespace='ns1', name='name1')], - False) - + False), + (matchers.MatchRepositoryToRequirementSpec, + ([RS()],), + [CR(namespace='ns1', name='name1')], + False), + (matchers.MatchRepositoryToRequirementSpec, + ([RS(namespace='ns1', name='name1', version_spec='*')], ), + [CR(namespace='ns1', name='name1')], + True) ]) def test_match_all(matcher_class, matcher_args, candidates, expected): log.debug('matcher_class=%s matcher_args=%s candidate=%s, expected=%s', matcher_class, matcher_args, candidates, expected) From d4a601653635bb2015ab63b5abd5d55210a3103c Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Wed, 22 May 2019 15:13:17 -0400 Subject: [PATCH 06/17] Add missing irdb param for failing test --- tests/ansible_galaxy/actions/test_install_action.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/ansible_galaxy/actions/test_install_action.py b/tests/ansible_galaxy/actions/test_install_action.py index ccf8beff..955bdf7c 100644 --- a/tests/ansible_galaxy/actions/test_install_action.py +++ b/tests/ansible_galaxy/actions/test_install_action.py @@ -143,7 +143,10 @@ def test_install_repository_deprecated(galaxy_context, mocker): mock_display_callback = mocker.MagicMock(name='mock_display_callback') + irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) + ret = install.install_repository(galaxy_context, + irdb, requirement_to_install=requirements_to_install[0], display_callback=mock_display_callback) From 33460a6009539fc7db75017b1d33a3047f78ac15 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Wed, 22 May 2019 16:18:18 -0400 Subject: [PATCH 07/17] More testings of install action kinda sorta emulate a dep solving iteration --- .../actions/test_install_action.py | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/ansible_galaxy/actions/test_install_action.py b/tests/ansible_galaxy/actions/test_install_action.py index 955bdf7c..3241439f 100644 --- a/tests/ansible_galaxy/actions/test_install_action.py +++ b/tests/ansible_galaxy/actions/test_install_action.py @@ -36,6 +36,80 @@ def test_requirements_from_strings(): assert 'testuser' in namespaces +def test_install_repository_specs_loop(galaxy_context, mocker): + repo_spec = RepositorySpec(namespace='alikins', name='some_collection', + version='4.2.1') + repo = Repository(repository_spec=repo_spec, installed=True) + + # needy_repo_spec = RepositorySpec(namespace='needy', + # name='allie', + # version='9.1.1'), + + other_repo_spec = RepositorySpec(namespace='testuser', + name='another', + version='11.12.99') + + needy_req_spec = RequirementSpec(namespace='some_required_namespace', + name='some_required_name', + version_spec='==1.0.0') + + needy_requirement = Requirement(repository_spec=other_repo_spec, + op=RequirementOps.EQ, + requirement_spec=needy_req_spec) + + other_repo = Repository(repository_spec=other_repo_spec, + requirements=[needy_requirement], + installed=True) + + needed_repo_spec = RepositorySpec(namespace='some_required_namespace', + name='some_required_name', + version='1.0.0') + + needed_repo = Repository(repository_spec=needed_repo_spec, + requirements=[], + ) + + requested_spec_strings = install.requirements_from_strings(['alikins.some_collection', + 'testuser.another']) + + log.debug('req_spec_strings: %s', requested_spec_strings) + requirements_list = requested_spec_strings + # install.requirements_from_strings(repository_spec_strings=requested_spec_strings, + # editable=False, + # namespace_override=None) + + import pprint + + def mock_install_repository(*args, **kwargs): + log.debug('mir: args=%s, kwargs=%s', pprint.pformat(args), repr(kwargs)) + req = args[2] + + log.debug('install_repo req: %s', req) + log.debug('install_repo reqlabel: %s', req.requirement_spec.label) + + # TODO: fix .label / add .label equilv sans version + + repos = {'some_collection': [repo], + 'another': [other_repo], + 'some_required_name': [needed_repo]} + return repos[req.requirement_spec.name] + + mock_ir = mocker.patch('ansible_galaxy.actions.install.install_repository', + side_effect=mock_install_repository) + + res = install.install_repository_specs_loop(galaxy_context, + requirements_list, + display_callback=display_callback) + + log.debug('res: %s', res) + log.debug('mock_ir.call_args_list: %s', pprint.pformat(mock_ir.call_args_list)) + + for call in mock_ir.call_args_list: + log.debug('call: %s', pprint.pformat(list(call))) + + assert res == 0 # duh, it's hardcoded + + def test_install_requirements(galaxy_context, mocker): repo_spec = RepositorySpec(namespace='alikins', name='some_collection', version='4.2.1') From 879522718f9abd26e7c5c2a0aeb6044ec28b63e5 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Wed, 22 May 2019 17:15:17 -0400 Subject: [PATCH 08/17] reqs_from_strings->reqs, reorder install action Move requirements_from_strings() from ansible_galaxy.actions.install to ansible_galaxy.requirements with from_requirements_dict Reorder the methods in ansible_galaxy.actions.install based on order of execution instead of the other style which we'll call... aleatoric? Add test_requirements unit tests --- ansible_galaxy/actions/install.py | 471 ++++++++---------- ansible_galaxy/requirements.py | 60 ++- ansible_galaxy_cli/cli/galaxy.py | 11 +- .../actions/test_install_action.py | 40 +- tests/ansible_galaxy/test_requirements.py | 75 +++ 5 files changed, 354 insertions(+), 303 deletions(-) create mode 100644 tests/ansible_galaxy/test_requirements.py diff --git a/ansible_galaxy/actions/install.py b/ansible_galaxy/actions/install.py index 0eccbd6f..7a056c80 100644 --- a/ansible_galaxy/actions/install.py +++ b/ansible_galaxy/actions/install.py @@ -1,18 +1,12 @@ import logging import pprint -from ansible_galaxy import collection_artifact from ansible_galaxy import display -from ansible_galaxy import download from ansible_galaxy import exceptions from ansible_galaxy import install from ansible_galaxy import installed_repository_db from ansible_galaxy import matchers -from ansible_galaxy import repository_spec_parse from ansible_galaxy.fetch import fetch_factory -from ansible_galaxy.models.repository_spec import FetchMethods -from ansible_galaxy.models.requirement import Requirement, RequirementOps -from ansible_galaxy.models.requirement_spec import RequirementSpec log = logging.getLogger(__name__) @@ -46,6 +40,209 @@ def _verify_requirements_repository_spec_have_namespaces(requirements_list): repository_spec=req_spec) +def install_repository(galaxy_context, + irdb, + requirement_to_install, + display_callback=None, + # TODO: error handling callback ? + ignore_errors=False, + no_deps=False, + force_overwrite=False): + '''This installs a single package by finding it, fetching it, verifying it and installing it.''' + + display_callback = display_callback or display.display_callback + + # INITIAL state + # dep_requirements = [] + + # TODO: we could do all the downloads first, then install them. Likely + # less error prone mid 'transaction' + log.debug('Processing %r', requirement_to_install) + + repository_spec_to_install = requirement_to_install.requirement_spec + requirement_spec_to_install = requirement_to_install.requirement_spec + + # else trans to ... FIND_FETCHER? + + # TODO: check if already installed and move to approriate state + + log.debug('About to find() requested requirement_spec_to_install: %s', requirement_spec_to_install) + + # potential_repository_spec is a repo spec for the install candidate we potentially found. + log.debug('Checking to see if %s is already installed', requirement_spec_to_install) + + already_installed_iter = irdb.by_requirement_spec(requirement_spec_to_install) + already_installed = sorted(list(already_installed_iter)) + + log.debug('req_spec: %s already_installed: %s', requirement_spec_to_install, already_installed) + + if already_installed: + for already_installed_repository in already_installed: + display_callback('%s is already installed at %s' % (already_installed_repository.repository_spec.label, + already_installed_repository.path), + level='warning') + log.debug('Stuff %s was already installed. In %s', requirement_spec_to_install, already_installed) + + return None + + # We dont have anything that matches the RequirementSpec installed + fetcher = fetch_factory.get(galaxy_context=galaxy_context, + requirement_spec=requirement_spec_to_install) + + # if we fail to get a fetcher here, then to... FIND_FETCHER_FAILURE ? + # could also move some of the logic in fetcher_factory to be driven from here + # and make the steps of mapping repository spec -> fetcher method part of the + # state machine. That might be a good place to support multiple galaxy servers + # or preferring local content to remote content, etc. + + # FIND state + # See if we can find metadata and/or download the archive before we try to + # remove an installed version... + try: + find_results = install.find(fetcher) + except exceptions.GalaxyError as e: + log.debug('requirement_to_install %s failed to be met: %s', requirement_to_install, e) + log.warning('Unable to find metadata for %s: %s', requirement_spec_to_install.label, e) + # FIXME: raise dep error exception? + raise_without_ignore(ignore_errors, e) + + # continue + return None + + # TODO: make sure repository_spec version is correct and set + + # TODO: state transition, if find_results -> INSTALL + # if not, then FIND_FAILED + + # TODO/FIXME: We give find() a RequirementSpec, but find_results should have enough + # info to create a concrete RepositorySpec + + # TODO: if we want client side content whitelist/blacklist, or pinned versions, + # or rules to only update within some semver range (ie, only 'patch' level), + # we could hook rule validation stuff here. + + # TODO: build a new repository_spec based on what we actually fetched to feed to + # install etc. The fetcher.fetch() could return a datastructure needed to build + # the new one instead of doing it in verify() + found_repository_spec = install.repository_spec_from_find_results(find_results, + requirement_spec_to_install) + + log.debug('found_repository_spec: %s', found_repository_spec) + + repository_spec_to_install = found_repository_spec + log.debug('About to download repository requested by %s: %s', requirement_spec_to_install, repository_spec_to_install) + + if find_results['custom'].get('collection_is_deprecated', False): + display_callback("The collection '%s' is deprecated." % (found_repository_spec.label), + level='warning') + + # FETCH state + try: + fetch_results = install.fetch(fetcher, + repository_spec=repository_spec_to_install, + find_results=find_results) + log.debug('fetch_results: %s', fetch_results) + # fetch_results will include a 'archive_path' pointing to where the artifact + # was saved to locally. + except exceptions.GalaxyError as e: + # fetch error probably should just go to a FAILED state, at least until + # we have to implement retries + log.warning('Unable to fetch %s: %s', repository_spec_to_install.name, e) + raise_without_ignore(ignore_errors, e) + # continue + # FIXME: raise ? + return None + + # FIXME: seems like we want to resolve deps before trying install + # We need the role (or other content) deps from meta before installing + # though, and sometimes (for galaxy case) we dont know that until we've downloaded + # the file, which we dont do until somewhere in the begin of content.install (fetch). + # We can get that from the galaxy API though. + # + # FIXME: exc handling + + installed_repositories = [] + + try: + installed_repositories = install.install(galaxy_context, + fetcher, + fetch_results, + repository_spec=found_repository_spec, + force_overwrite=force_overwrite, + display_callback=display_callback) + except exceptions.GalaxyError as e: + msg = "- %s was NOT installed successfully: %s " + display_callback(msg % (found_repository_spec.label, e), level='warning') + log.warning(msg, found_repository_spec.label, str(e)) + raise_without_ignore(ignore_errors, e) + return [] + + if not installed_repositories: + log.warning("- %s was NOT installed successfully.", found_repository_spec.label) + raise_without_ignore(ignore_errors) + + return installed_repositories + + +# TODO: split into resolve, find/get metadata, resolve deps, download, install transaction +def install_repositories(galaxy_context, + irdb, + requirements_to_install, + display_callback=None, + # TODO: error handling callback ? + ignore_errors=False, + no_deps=False, + force_overwrite=False): + + display_callback = display_callback or display.display_callback + log.debug('requirements_to_install: %s', requirements_to_install) + # log.debug('no_deps: %s', no_deps) + # log.debug('force_overwrite: %s', force_overwrite) + + # dep_requirements = [] + most_installed_repositories = [] + + # TODO: this should be adding the content/self.args/content_left to + # a list of needed deps + + # Remove any dupe repository_specs + requirements_to_install_uniq = set(requirements_to_install) + + # TODO: if the default ordering of repository_specs isnt useful, may need to tweak it + for requirement_to_install in sorted(requirements_to_install_uniq): + log.debug('requirement_to_install: %s', requirement_to_install) + + installed_repositories = install_repository(galaxy_context, + irdb, + requirement_to_install, + display_callback=display_callback, + ignore_errors=ignore_errors, + no_deps=no_deps, + force_overwrite=force_overwrite) + + # log.debug('dep_requirement_repository_specs1: %s', dep_requirements) + + if not installed_repositories: + log.debug('install_repository() returned None for requirement_to_install: %s', requirement_to_install) + continue + + for installed_repo in installed_repositories: + required_by_blurb = '' + if requirement_to_install.repository_spec: + required_by_blurb = ' (required by %s)' % requirement_to_install.repository_spec.label + + log.info('Installed: %s %s to %s%s', + installed_repo.label, + installed_repo.repository_spec.version, + installed_repo.path, + required_by_blurb) + + most_installed_repositories.extend(installed_repositories) + # dep_requirements.extend(new_dep_requirements) + + return most_installed_repositories + + # pass a list of repository_spec objects def install_requirements(galaxy_context, irdb, @@ -55,7 +252,10 @@ def install_requirements(galaxy_context, ignore_errors=False, no_deps=False, force_overwrite=False): - '''Install a set of repositories specified by repository_specs if they are not already installed''' + '''Install a set of repositories specified by requirements_list if they are not already installed + + requirements_list is a list of Requirement() instances. + ''' log.debug('requirements_list: %s', requirements_list) @@ -95,55 +295,6 @@ def install_requirements(galaxy_context, force_overwrite=force_overwrite) -def requirements_from_strings(repository_spec_strings, - namespace_override=None, - editable=False): - requirements_list = [] - - for repository_spec_string in repository_spec_strings: - fetch_method = \ - repository_spec_parse.choose_repository_fetch_method(repository_spec_string, - editable=editable) - log.debug('fetch_method: %s', fetch_method) - - if fetch_method == FetchMethods.LOCAL_FILE: - # Since we only know this is a local file we vaguely recognize, we have to - # open it up to get any more details. We _could_ attempt to parse the file - # name, but that rarely ends well. Filename could also be arbitrary for downloads - # from remote urls ('mazer install http://myci.example.com/somebuildjob/latest' etc) - spec_data = collection_artifact.load_data_from_collection_artifact(repository_spec_string) - spec_data['fetch_method'] = fetch_method - elif fetch_method == FetchMethods.REMOTE_URL: - # download the url - # hope it is a collection artifact and use load_data_from_collection_artifact() for the - # rest of the repo_spec data - log.debug('repository_spec_string: %s', repository_spec_string) - - tmp_downloaded_path = download.fetch_url(repository_spec_string, - # This is for random remote_urls, so always validate_certs - validate_certs=True) - spec_data = collection_artifact.load_data_from_collection_artifact(tmp_downloaded_path) - - # pretend like this is a local_file install now - spec_data['fetch_method'] = FetchMethods.LOCAL_FILE - else: - spec_data = repository_spec_parse.spec_data_from_string(repository_spec_string, - namespace_override=namespace_override, - editable=editable) - - spec_data['fetch_method'] = fetch_method - - log.debug('spec_data: %s', spec_data) - - req_spec = RequirementSpec.from_dict(spec_data) - - req = Requirement(repository_spec=None, op=RequirementOps.EQ, requirement_spec=req_spec) - - requirements_list.append(req) - - return requirements_list - - # NOTE: this is equiv to add deps to a transaction def find_new_deps_from_installed(galaxy_context, installed_repos, no_deps=False): if no_deps: @@ -250,211 +401,3 @@ def install_repository_specs_loop(galaxy_context, # FIXME: what results to return? return 0 - - -# TODO: split into resolve, find/get metadata, resolve deps, download, install transaction -def install_repositories(galaxy_context, - irdb, - requirements_to_install, - display_callback=None, - # TODO: error handling callback ? - ignore_errors=False, - no_deps=False, - force_overwrite=False): - - display_callback = display_callback or display.display_callback - log.debug('requirements_to_install: %s', requirements_to_install) - # log.debug('no_deps: %s', no_deps) - # log.debug('force_overwrite: %s', force_overwrite) - - # dep_requirements = [] - most_installed_repositories = [] - - # TODO: this should be adding the content/self.args/content_left to - # a list of needed deps - - # Remove any dupe repository_specs - requirements_to_install_uniq = set(requirements_to_install) - - # TODO: if the default ordering of repository_specs isnt useful, may need to tweak it - for requirement_to_install in sorted(requirements_to_install_uniq): - log.debug('requirement_to_install: %s', requirement_to_install) - - installed_repositories = install_repository(galaxy_context, - irdb, - requirement_to_install, - display_callback=display_callback, - ignore_errors=ignore_errors, - no_deps=no_deps, - force_overwrite=force_overwrite) - - # log.debug('dep_requirement_repository_specs1: %s', dep_requirements) - - if not installed_repositories: - log.debug('install_repository() returned None for requirement_to_install: %s', requirement_to_install) - continue - - for installed_repo in installed_repositories: - required_by_blurb = '' - if requirement_to_install.repository_spec: - required_by_blurb = ' (required by %s)' % requirement_to_install.repository_spec.label - - log.info('Installed: %s %s to %s%s', - installed_repo.label, - installed_repo.repository_spec.version, - installed_repo.path, - required_by_blurb) - - most_installed_repositories.extend(installed_repositories) - # dep_requirements.extend(new_dep_requirements) - - return most_installed_repositories - - -def install_repository(galaxy_context, - irdb, - requirement_to_install, - display_callback=None, - # TODO: error handling callback ? - ignore_errors=False, - no_deps=False, - force_overwrite=False): - '''This installs a single package by finding it, fetching it, verifying it and installing it.''' - - display_callback = display_callback or display.display_callback - - # INITIAL state - # dep_requirements = [] - - # TODO: we could do all the downloads first, then install them. Likely - # less error prone mid 'transaction' - log.debug('Processing %r', requirement_to_install) - - repository_spec_to_install = requirement_to_install.requirement_spec - requirement_spec_to_install = requirement_to_install.requirement_spec - - # else trans to ... FIND_FETCHER? - - # TODO: check if already installed and move to approriate state - - log.debug('About to find() requested requirement_spec_to_install: %s', requirement_spec_to_install) - - # potential_repository_spec is a repo spec for the install candidate we potentially found. - log.debug('Checking to see if %s is already installed', requirement_spec_to_install) - - already_installed_iter = irdb.by_requirement_spec(requirement_spec_to_install) - already_installed = sorted(list(already_installed_iter)) - - log.debug('req_spec: %s already_installed: %s', requirement_spec_to_install, already_installed) - - if already_installed: - for already_installed_repository in already_installed: - display_callback('%s is already installed at %s' % (already_installed_repository.repository_spec.label, - already_installed_repository.path), - level='warning') - log.debug('Stuff %s was already installed. In %s', requirement_spec_to_install, already_installed) - - return None - - # TODO: The already installed check above verifies that nothing that matches the requirement spec is installed, - # but just because the name+version required wasn't installed, that doesn't mean that name at a different - # version isn't installed. - # To catch that, also need to check if the irdb by name to see if anything with that name is installed. - # - # We dont have anything that matches the RequirementSpec installed - fetcher = fetch_factory.get(galaxy_context=galaxy_context, - requirement_spec=requirement_spec_to_install) - - # if we fail to get a fetcher here, then to... FIND_FETCHER_FAILURE ? - # could also move some of the logic in fetcher_factory to be driven from here - # and make the steps of mapping repository spec -> fetcher method part of the - # state machine. That might be a good place to support multiple galaxy servers - # or preferring local content to remote content, etc. - - # FIND state - # See if we can find metadata and/or download the archive before we try to - # remove an installed version... - try: - find_results = install.find(fetcher) - except exceptions.GalaxyError as e: - log.debug('requirement_to_install %s failed to be met: %s', requirement_to_install, e) - log.warning('Unable to find metadata for %s: %s', requirement_spec_to_install.label, e) - # FIXME: raise dep error exception? - raise_without_ignore(ignore_errors, e) - - # continue - return None - - # TODO: make sure repository_spec version is correct and set - - # TODO: state transition, if find_results -> INSTALL - # if not, then FIND_FAILED - - # TODO/FIXME: We give find() a RequirementSpec, but find_results should have enough - # info to create a concrete RepositorySpec - - # TODO: if we want client side content whitelist/blacklist, or pinned versions, - # or rules to only update within some semver range (ie, only 'patch' level), - # we could hook rule validation stuff here. - - # TODO: build a new repository_spec based on what we actually fetched to feed to - # install etc. The fetcher.fetch() could return a datastructure needed to build - # the new one instead of doing it in verify() - found_repository_spec = install.repository_spec_from_find_results(find_results, - requirement_spec_to_install) - - log.debug('found_repository_spec: %s', found_repository_spec) - - repository_spec_to_install = found_repository_spec - log.debug('About to download repository requested by %s: %s', requirement_spec_to_install, repository_spec_to_install) - - if find_results['custom'].get('collection_is_deprecated', False): - display_callback("The collection '%s' is deprecated." % (found_repository_spec.label), - level='warning') - - # FETCH state - try: - fetch_results = install.fetch(fetcher, - repository_spec=repository_spec_to_install, - find_results=find_results) - log.debug('fetch_results: %s', fetch_results) - # fetch_results will include a 'archive_path' pointing to where the artifact - # was saved to locally. - except exceptions.GalaxyError as e: - # fetch error probably should just go to a FAILED state, at least until - # we have to implement retries - log.warning('Unable to fetch %s: %s', repository_spec_to_install.name, e) - raise_without_ignore(ignore_errors, e) - # continue - # FIXME: raise ? - return None - - # FIXME: seems like we want to resolve deps before trying install - # We need the role (or other content) deps from meta before installing - # though, and sometimes (for galaxy case) we dont know that until we've downloaded - # the file, which we dont do until somewhere in the begin of content.install (fetch). - # We can get that from the galaxy API though. - # - # FIXME: exc handling - - installed_repositories = [] - - try: - installed_repositories = install.install(galaxy_context, - fetcher, - fetch_results, - repository_spec=found_repository_spec, - force_overwrite=force_overwrite, - display_callback=display_callback) - except exceptions.GalaxyError as e: - msg = "- %s was NOT installed successfully: %s " - display_callback(msg % (found_repository_spec, e), level='warning') - log.warning(msg, found_repository_spec.label, str(e)) - raise_without_ignore(ignore_errors, e) - return [] - - if not installed_repositories: - log.warning("- %s was NOT installed successfully.", found_repository_spec.label) - raise_without_ignore(ignore_errors) - - return installed_repositories diff --git a/ansible_galaxy/requirements.py b/ansible_galaxy/requirements.py index 8de51d3f..ba2eb9ec 100644 --- a/ansible_galaxy/requirements.py +++ b/ansible_galaxy/requirements.py @@ -1,19 +1,71 @@ import logging +from ansible_galaxy import collection_artifact +from ansible_galaxy import download +from ansible_galaxy.models.repository_spec import FetchMethods from ansible_galaxy.models.requirement import Requirement, RequirementOps, RequirementScopes from ansible_galaxy.models.requirement_spec import RequirementSpec -from ansible_galaxy.repository_spec_parse import spec_data_from_string +from ansible_galaxy import repository_spec_parse log = logging.getLogger(__name__) +def requirements_from_strings(repository_spec_strings, + namespace_override=None, + editable=False): + requirements_list = [] + + for repository_spec_string in repository_spec_strings: + fetch_method = \ + repository_spec_parse.choose_repository_fetch_method(repository_spec_string, + editable=editable) + log.debug('fetch_method: %s', fetch_method) + + if fetch_method == FetchMethods.LOCAL_FILE: + # Since we only know this is a local file we vaguely recognize, we have to + # open it up to get any more details. We _could_ attempt to parse the file + # name, but that rarely ends well. Filename could also be arbitrary for downloads + # from remote urls ('mazer install http://myci.example.com/somebuildjob/latest' etc) + spec_data = collection_artifact.load_data_from_collection_artifact(repository_spec_string) + spec_data['fetch_method'] = fetch_method + elif fetch_method == FetchMethods.REMOTE_URL: + # download the url + # hope it is a collection artifact and use load_data_from_collection_artifact() for the + # rest of the repo_spec data + log.debug('repository_spec_string: %s', repository_spec_string) + + tmp_downloaded_path = download.fetch_url(repository_spec_string, + # This is for random remote_urls, so always validate_certs + validate_certs=True) + spec_data = collection_artifact.load_data_from_collection_artifact(tmp_downloaded_path) + + # pretend like this is a local_file install now + spec_data['fetch_method'] = FetchMethods.LOCAL_FILE + else: + spec_data = repository_spec_parse.spec_data_from_string(repository_spec_string, + namespace_override=namespace_override, + editable=editable) + + spec_data['fetch_method'] = fetch_method + + log.debug('spec_data: %s', spec_data) + + req_spec = RequirementSpec.from_dict(spec_data) + + req = Requirement(repository_spec=None, op=RequirementOps.EQ, requirement_spec=req_spec) + + requirements_list.append(req) + + return requirements_list + + def from_dependencies_dict(dependencies_dict, namespace_override=None, editable=False, repository_spec=None): '''Build a list of Requirement objects from the 'dependencies' item in galaxy.yml''' reqs = [] for req_label, req_version_spec in dependencies_dict.items(): - req_spec_data = spec_data_from_string(req_label, - namespace_override=namespace_override, - editable=editable) + req_spec_data = repository_spec_parse.spec_data_from_string(req_label, + namespace_override=namespace_override, + editable=editable) req_spec_data['version_spec'] = req_version_spec log.debug('req_spec_data: %s', req_spec_data) diff --git a/ansible_galaxy_cli/cli/galaxy.py b/ansible_galaxy_cli/cli/galaxy.py index 0bd1b53e..e7e14ed0 100644 --- a/ansible_galaxy_cli/cli/galaxy.py +++ b/ansible_galaxy_cli/cli/galaxy.py @@ -39,6 +39,7 @@ from ansible_galaxy.config import config from ansible_galaxy import matchers +from ansible_galaxy import requirements from ansible_galaxy import rest_api from ansible_galaxy.models.context import GalaxyContext @@ -275,14 +276,14 @@ def execute_install(self): galaxy_context = self._get_galaxy_context(self.options, self.config) requested_spec_strings = self.args - requirements = \ - install.requirements_from_strings(repository_spec_strings=requested_spec_strings, - editable=self.options.editable_install, - namespace_override=self.options.namespace) + requirements_list = \ + requirements.requirements_from_strings(repository_spec_strings=requested_spec_strings, + editable=self.options.editable_install, + namespace_override=self.options.namespace) # TODO: build requirement_specs from requested_collection_specs strings rc = install.install_repository_specs_loop(galaxy_context, - requirements, + requirements_list, display_callback=self.display, ignore_errors=self.options.ignore_errors, no_deps=self.options.no_deps, diff --git a/tests/ansible_galaxy/actions/test_install_action.py b/tests/ansible_galaxy/actions/test_install_action.py index 3241439f..5c29c65c 100644 --- a/tests/ansible_galaxy/actions/test_install_action.py +++ b/tests/ansible_galaxy/actions/test_install_action.py @@ -18,24 +18,6 @@ def display_callback(msg, **kwargs): log.debug(msg) -def test_requirements_from_strings(): - # TODO: tests for local file, remote url, etc - res = install.requirements_from_strings(['alikins.some_collection', - 'testuser.another', - ]) - - log.debug('res: %s', res) - - assert isinstance(res, list) - reqs = [req for req in res] - for req in reqs: - assert isinstance(req, Requirement) - - namespaces = [x.requirement_spec.namespace for x in res] - assert 'alikins' in namespaces - assert 'testuser' in namespaces - - def test_install_repository_specs_loop(galaxy_context, mocker): repo_spec = RepositorySpec(namespace='alikins', name='some_collection', version='4.2.1') @@ -69,18 +51,14 @@ def test_install_repository_specs_loop(galaxy_context, mocker): requirements=[], ) - requested_spec_strings = install.requirements_from_strings(['alikins.some_collection', + requirements_list = requirements.requirements_from_strings(['alikins.some_collection', 'testuser.another']) - log.debug('req_spec_strings: %s', requested_spec_strings) - requirements_list = requested_spec_strings - # install.requirements_from_strings(repository_spec_strings=requested_spec_strings, - # editable=False, - # namespace_override=None) + log.debug('requirements_list: %s', requirements_list) import pprint - def mock_install_repository(*args, **kwargs): + def stub_install_repository(*args, **kwargs): log.debug('mir: args=%s, kwargs=%s', pprint.pformat(args), repr(kwargs)) req = args[2] @@ -92,10 +70,12 @@ def mock_install_repository(*args, **kwargs): repos = {'some_collection': [repo], 'another': [other_repo], 'some_required_name': [needed_repo]} + return repos[req.requirement_spec.name] + # mock out the install_repository to avoid the network requests, etc mock_ir = mocker.patch('ansible_galaxy.actions.install.install_repository', - side_effect=mock_install_repository) + side_effect=stub_install_repository) res = install.install_repository_specs_loop(galaxy_context, requirements_list, @@ -120,10 +100,10 @@ def test_install_requirements(galaxy_context, mocker): version='11.12.99'), installed=True) - requirements = install.requirements_from_strings(['alikins.some_collection', - 'testuser.another']) + requirements_list = requirements.requirements_from_strings(['alikins.some_collection', + 'testuser.another']) - log.debug('requirements: %s', requirements) + log.debug('requirements_list: %s', requirements_list) mock_irdb2 = mocker.MagicMock(name='the_mock_irdb2') mock_irdb2.select.return_value = [repo, other_repo] @@ -139,7 +119,7 @@ def test_install_requirements(galaxy_context, mocker): res = install.install_requirements(galaxy_context, mock_irdb2, - requirements, + requirements_list, display_callback=None, ignore_errors=False, no_deps=False, diff --git a/tests/ansible_galaxy/test_requirements.py b/tests/ansible_galaxy/test_requirements.py new file mode 100644 index 00000000..84c108ca --- /dev/null +++ b/tests/ansible_galaxy/test_requirements.py @@ -0,0 +1,75 @@ +import logging + +from semantic_version import Version, Spec + +from ansible_galaxy import requirements +from ansible_galaxy.models.requirement import Requirement +from ansible_galaxy.models.requirement_spec import RequirementSpec + +log = logging.getLogger(__name__) + + +def test_from_dependencies_dict_empty(): + dep_dict = {} + res = requirements.from_dependencies_dict(dep_dict) + + log.debug('res: %s', res) + + assert isinstance(res, list) + + +def test_from_dependencies_dict(): + dep_dict = {'alikins.some_collection': '>=1.0.0', + 'testuser.another': '==0.6.6', + 'shrug.whatever': '*'} + res = requirements.from_dependencies_dict(dep_dict) + + log.debug('res: %s', res) + + assert isinstance(res, list) + for req in res: + assert isinstance(req, Requirement) + + log.debug('res[0]: %s', res[0]) + assert isinstance(res[0].requirement_spec, RequirementSpec) + assert res[0].requirement_spec.namespace == 'alikins' + assert res[0].requirement_spec.name == 'some_collection' + assert res[0].requirement_spec.version_spec == Spec('>=1.0.0') + assert res[0].requirement_spec.version_spec.match(Version('1.0.0')) + assert res[0].requirement_spec.version_spec.match(Version('1.2.0')) + assert not res[0].requirement_spec.version_spec.match(Version('0.0.37')) + + +def test_requirements_from_strings(): + # TODO: tests for local file, remote url, etc + res = requirements.requirements_from_strings(['alikins.some_collection', + 'alikins.picky,1.2.3', + 'alikins.picky,version=3.4.5', + 'testuser.another', + # FIXME: '=' and ',' in version spec + # freaks out cli parser + # 'testuser.picky,version="!=3.1.4,>=3.0.0"', + ]) + + log.debug('res: %s', res) + + assert isinstance(res, list) + reqs = [req for req in res] + for req in reqs: + assert isinstance(req, Requirement) + + namespaces = [x.requirement_spec.namespace for x in res] + assert 'alikins' in namespaces + assert 'testuser' in namespaces + assert res[0].requirement_spec.namespace == 'alikins' + assert res[0].requirement_spec.name == 'some_collection' + assert res[0].requirement_spec.version_spec == Spec('*') + + assert res[1].requirement_spec.namespace == 'alikins' + assert res[1].requirement_spec.name == 'picky' + assert res[1].requirement_spec.version_spec == Spec('==1.2.3') + + assert res[2].requirement_spec.namespace == 'alikins' + assert res[2].requirement_spec.name == 'picky' + assert res[2].requirement_spec.version_spec == Spec('==3.4.5') + From fa1dc41f6307acbfc6f2683824154db5a779e3fc Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Wed, 22 May 2019 17:39:34 -0400 Subject: [PATCH 09/17] find_new_deps... -> find_new_requirements... There is no longer the separate concept of 'dependencies' (install time) and 'requirements' (run time... and install time). There are only 'dependencies' now. Which we call 'requirements' almost everywhere inside the code because reasons. But for the moment, at least consistently call them by the wrong name everywhere, so they can all be changed at the same time. --- ansible_galaxy/actions/install.py | 41 +++++++++---------- .../actions/test_install_action.py | 6 +-- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/ansible_galaxy/actions/install.py b/ansible_galaxy/actions/install.py index 7a056c80..9223f453 100644 --- a/ansible_galaxy/actions/install.py +++ b/ansible_galaxy/actions/install.py @@ -296,12 +296,11 @@ def install_requirements(galaxy_context, # NOTE: this is equiv to add deps to a transaction -def find_new_deps_from_installed(galaxy_context, installed_repos, no_deps=False): +def find_new_requirements_from_installed(galaxy_context, installed_repos, no_deps=False): if no_deps: return [] - # FIXME: Just return the single item list installed_repositories here - deps_and_reqs_set = set() + total_reqs_set = set() log.debug('finding new deps for installed repos: %s', [str(x) for x in installed_repos]) @@ -313,43 +312,43 @@ def find_new_deps_from_installed(galaxy_context, installed_repos, no_deps=False) for installed_repository in installed_repos: # log.debug('just_installed_repository: %s', installed_repository) - # convert deps/reqs to sets. Losing any ordering, but avoids dupes + # convert reqs to sets. Losing any ordering, but avoids dupes reqs_set = set(installed_repository.requirements) - deps_and_reqs_set.update(reqs_set) + total_reqs_set.update(reqs_set) - # for dep_req in sorted(deps_and_reqs_set): - # log.debug('deps_and_reqs_set_item: %s', dep_req) + # TODO: This does not grow nicely as the size + # of the list of requirements of everything installed grows + all_requirements = sorted(list(total_reqs_set)) - deps_and_reqs_list = sorted(list(deps_and_reqs_set)) + # log.debug('reqs_list: %s', pf(reqs_list)) - # log.debug('deps_and_reqs_list: %s', pf(deps_and_reqs_list)) + unsolved_requirements = [] - unsolved_deps_reqs = [] - for dep_req in deps_and_reqs_list: - log.debug('Checking if %s is provided by something installed', str(dep_req)) + for requirement in all_requirements: + log.debug('Checking if %s is provided by something installed', str(requirement)) # Search for an exact ns_n_v match irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) - already_installed_iter = irdb.by_requirement(dep_req) + already_installed_iter = irdb.by_requirement(requirement) already_installed = list(already_installed_iter) log.debug('already_installed: %s', already_installed) solved = False for provider in already_installed: - log.debug('The dep_req %s is already provided by %s', dep_req, provider) + log.debug('The requirement %s is already provided by %s', requirement, provider) solved = solved or True if solved: - log.debug('skipping dep_req %s', dep_req) + log.debug('skipping requirement %s', requirement) continue - unsolved_deps_reqs.append(dep_req) + unsolved_requirements.append(requirement) - log.debug('Found additional requirements: %s', pprint.pformat(unsolved_deps_reqs)) + log.debug('Found additional requirements: %s', pprint.pformat(unsolved_requirements)) - return unsolved_deps_reqs + return unsolved_requirements # FIXME: probably pass the point where passing around all the data to methods makes sense @@ -388,9 +387,9 @@ def install_repository_specs_loop(galaxy_context, # set the repository_specs to search for to whatever the install reported as being needed yet # requirements_list = new_requirements_list - requirements_list = find_new_deps_from_installed(galaxy_context, - just_installed_repositories, - no_deps=no_deps) + requirements_list = find_new_requirements_from_installed(galaxy_context, + just_installed_repositories, + no_deps=no_deps) for req in requirements_list: if req.repository_spec: diff --git a/tests/ansible_galaxy/actions/test_install_action.py b/tests/ansible_galaxy/actions/test_install_action.py index 5c29c65c..e8eeb33e 100644 --- a/tests/ansible_galaxy/actions/test_install_action.py +++ b/tests/ansible_galaxy/actions/test_install_action.py @@ -253,12 +253,12 @@ def test_verify_repository_specs_have_namespace(galaxy_context): def test_find_new_deps_from_installed_no_deps(galaxy_context): - res = install.find_new_deps_from_installed(galaxy_context, [], no_deps=True) + res = install.find_new_requirements_from_installed(galaxy_context, [], no_deps=True) assert res == [] def test_find_new_deps_from_installed_nothing_installed(galaxy_context): - res = install.find_new_deps_from_installed(galaxy_context, []) + res = install.find_new_requirements_from_installed(galaxy_context, []) assert res == [] @@ -276,7 +276,7 @@ def test_find_new_deps_from_installed(galaxy_context): requirement_spec=req_spec) installed_repo = Repository(repo_spec, requirements=[some_requirement, some_requirement]) - res = install.find_new_deps_from_installed(galaxy_context, [installed_repo]) + res = install.find_new_requirements_from_installed(galaxy_context, [installed_repo]) log.debug('res: %s', res) assert isinstance(res, list) From 7b6c52b1ac944f206f9b73196e2d4486f71ee50b Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Wed, 22 May 2019 17:52:13 -0400 Subject: [PATCH 10/17] comment cleanup, style --- ansible_galaxy/actions/install.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/ansible_galaxy/actions/install.py b/ansible_galaxy/actions/install.py index 9223f453..c1d6fcc6 100644 --- a/ansible_galaxy/actions/install.py +++ b/ansible_galaxy/actions/install.py @@ -252,7 +252,7 @@ def install_requirements(galaxy_context, ignore_errors=False, no_deps=False, force_overwrite=False): - '''Install a set of repositories specified by requirements_list if they are not already installed + '''Install a set of Collections specified by requirements_list if they are not already installed requirements_list is a list of Requirement() instances. ''' @@ -261,8 +261,6 @@ def install_requirements(galaxy_context, _verify_requirements_repository_spec_have_namespaces(requirements_list) - # FIXME: mv mv this filtering to it's own method - # match any of the content specs for stuff we want to install # TODO: this is part of building the install transaction # ie, see if it is already installed requested_repository_specs = [x.requirement_spec for x in requirements_list] @@ -274,12 +272,14 @@ def install_requirements(galaxy_context, # we can simplify this filter with set ops already_installed_repository_spec_set = set([installed.repository_spec for installed in already_installed_generator]) + log.debug('already_installed_repository_spec_set: %s', already_installed_repository_spec_set) - log.debug('force_overwrite: %s', force_overwrite) - # This filters out already installed repositories unless --force. Aside from the warning, 'mazer install alikins.something_installed_already' is ok. + # This filters out already installed repositories unless --force. + # Aside from the warning, 'mazer install alikins.something_installed_already' is ok. if force_overwrite: log.debug('--force/force_overwrite=True, so [re]installing everything in %s', requirements_list) + requirements_to_install = requirements_list else: requirements_to_install = [y for y in requirements_list if y.requirement_spec not in already_installed_repository_spec_set] @@ -305,14 +305,10 @@ def find_new_requirements_from_installed(galaxy_context, installed_repos, no_dep log.debug('finding new deps for installed repos: %s', [str(x) for x in installed_repos]) - # Remove dupes. Note, can/will change ordering. - # installed_repos = list(set(installed_repos)) - - # install requirements ("dependcies" in collection info), if we want them + # install requirements ("dependencies" in collection info), if we want them for installed_repository in installed_repos: - # log.debug('just_installed_repository: %s', installed_repository) - # convert reqs to sets. Losing any ordering, but avoids dupes + # convert reqs list to sets, Losing any ordering, but avoids dupes of requirements reqs_set = set(installed_repository.requirements) total_reqs_set.update(reqs_set) @@ -321,8 +317,6 @@ def find_new_requirements_from_installed(galaxy_context, installed_repos, no_dep # of the list of requirements of everything installed grows all_requirements = sorted(list(total_reqs_set)) - # log.debug('reqs_list: %s', pf(reqs_list)) - unsolved_requirements = [] for requirement in all_requirements: From cf6649e15a3ddd82da9690baf26834faaa94911f Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Wed, 22 May 2019 17:55:35 -0400 Subject: [PATCH 11/17] Rename install_repository_specs_loop->install_requirements_loop --- ansible_galaxy/actions/install.py | 14 +++++++------- ansible_galaxy_cli/cli/galaxy.py | 12 ++++++------ .../ansible_galaxy/actions/test_install_action.py | 6 +++--- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/ansible_galaxy/actions/install.py b/ansible_galaxy/actions/install.py index c1d6fcc6..a32b7db4 100644 --- a/ansible_galaxy/actions/install.py +++ b/ansible_galaxy/actions/install.py @@ -347,13 +347,13 @@ def find_new_requirements_from_installed(galaxy_context, installed_repos, no_dep # FIXME: probably pass the point where passing around all the data to methods makes sense # so probably needs a stateful class here -def install_repository_specs_loop(galaxy_context, - requirements, - display_callback=None, - # TODO: error handling callback ? - ignore_errors=False, - no_deps=False, - force_overwrite=False): +def install_requirements_loop(galaxy_context, + requirements, + display_callback=None, + # TODO: error handling callback ? + ignore_errors=False, + no_deps=False, + force_overwrite=False): requirements_list = requirements diff --git a/ansible_galaxy_cli/cli/galaxy.py b/ansible_galaxy_cli/cli/galaxy.py index e7e14ed0..b4d87caf 100644 --- a/ansible_galaxy_cli/cli/galaxy.py +++ b/ansible_galaxy_cli/cli/galaxy.py @@ -282,12 +282,12 @@ def execute_install(self): namespace_override=self.options.namespace) # TODO: build requirement_specs from requested_collection_specs strings - rc = install.install_repository_specs_loop(galaxy_context, - requirements_list, - display_callback=self.display, - ignore_errors=self.options.ignore_errors, - no_deps=self.options.no_deps, - force_overwrite=self.options.force) + rc = install.install_requirements_loop(galaxy_context, + requirements_list, + display_callback=self.display, + ignore_errors=self.options.ignore_errors, + no_deps=self.options.no_deps, + force_overwrite=self.options.force) return rc diff --git a/tests/ansible_galaxy/actions/test_install_action.py b/tests/ansible_galaxy/actions/test_install_action.py index e8eeb33e..691de873 100644 --- a/tests/ansible_galaxy/actions/test_install_action.py +++ b/tests/ansible_galaxy/actions/test_install_action.py @@ -77,9 +77,9 @@ def stub_install_repository(*args, **kwargs): mock_ir = mocker.patch('ansible_galaxy.actions.install.install_repository', side_effect=stub_install_repository) - res = install.install_repository_specs_loop(galaxy_context, - requirements_list, - display_callback=display_callback) + res = install.install_requirements_loop(galaxy_context, + requirements_list, + display_callback=display_callback) log.debug('res: %s', res) log.debug('mock_ir.call_args_list: %s', pprint.pformat(mock_ir.call_args_list)) From 13bee6a9d39a6726089f6a392d1181621bdd3b4a Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Thu, 23 May 2019 11:33:54 -0400 Subject: [PATCH 12/17] Add actions.install.run() for impl most of 'install' subcommand The ansible_galaxy_cli code now calls ansible_galaxy.actions.install.run() instead of install_whatever_loop() as before. Plan is to move other actions to similar api. A run() that more or less takes the cli args (strings). And a suitably named method in the action module that shows a more pythonic signature. For example, here run() takes a list of strings for requirement_spec_strings, while install_requirements_loop() takes a list of Requirement() objects. Also, run() returns only a return code suitable for sys.exit(), while the more pythonic method will return a results dict with a 'status' and an 'errors' list. --- ansible_galaxy/actions/install.py | 56 ++++++++++++++++++- ansible_galaxy_cli/cli/galaxy.py | 19 ++----- .../actions/test_install_action.py | 6 +- 3 files changed, 65 insertions(+), 16 deletions(-) diff --git a/ansible_galaxy/actions/install.py b/ansible_galaxy/actions/install.py index a32b7db4..230ddcc5 100644 --- a/ansible_galaxy/actions/install.py +++ b/ansible_galaxy/actions/install.py @@ -1,4 +1,5 @@ import logging +import os import pprint from ansible_galaxy import display @@ -6,6 +7,7 @@ from ansible_galaxy import install from ansible_galaxy import installed_repository_db from ansible_galaxy import matchers +from ansible_galaxy import requirements from ansible_galaxy.fetch import fetch_factory log = logging.getLogger(__name__) @@ -345,6 +347,7 @@ def find_new_requirements_from_installed(galaxy_context, installed_repos, no_dep return unsolved_requirements +# TODO: rename 'install' once we rename ansible_galaxy.install to ansible_galaxy.install_collection # FIXME: probably pass the point where passing around all the data to methods makes sense # so probably needs a stateful class here def install_requirements_loop(galaxy_context, @@ -355,6 +358,11 @@ def install_requirements_loop(galaxy_context, no_deps=False, force_overwrite=False): + results = { + 'errors': [], + 'success': True + } + requirements_list = requirements log.debug('requirements_list: %s', requirements_list) @@ -392,5 +400,49 @@ def install_requirements_loop(galaxy_context, msg = 'Installing requirement %s' % req.requirement_spec.label display_callback(msg, level='info') - # FIXME: what results to return? - return 0 + return results + + +def run(galaxy_context, + requirement_spec_strings=None, + requirement_specs_file=None, + editable=False, + namespace_override=False, + ignore_errors=False, + no_deps=False, + force_overwrite=False, + display_callback=None): + + requirements_list = [] + + if requirement_spec_strings: + requirements_list += \ + requirements.requirements_from_strings(repository_spec_strings=requirement_spec_strings, + editable=editable, + namespace_override=namespace_override) + + if requirement_specs_file: + # yaml load the file + # requirements_list += \ + # requirements.requirements_from_dict() + pass + + results = install_requirements_loop(galaxy_context, + requirements_list, + display_callback=display_callback, + ignore_errors=ignore_errors, + no_deps=no_deps, + force_overwrite=force_overwrite) + + log.debug('install results: %s', results) + + if results['errors']: + for error in results['errors']: + display_callback(error) + + if results['success']: + return os.EX_OK # 0 + + # TODO: finer grained return codes + + return os.EX_SOFTWARE # 70 diff --git a/ansible_galaxy_cli/cli/galaxy.py b/ansible_galaxy_cli/cli/galaxy.py index b4d87caf..e5c46185 100644 --- a/ansible_galaxy_cli/cli/galaxy.py +++ b/ansible_galaxy_cli/cli/galaxy.py @@ -39,7 +39,6 @@ from ansible_galaxy.config import config from ansible_galaxy import matchers -from ansible_galaxy import requirements from ansible_galaxy import rest_api from ansible_galaxy.models.context import GalaxyContext @@ -274,20 +273,14 @@ def execute_install(self): self.log.debug('self.options: %s', self.options) galaxy_context = self._get_galaxy_context(self.options, self.config) - requested_spec_strings = self.args - - requirements_list = \ - requirements.requirements_from_strings(repository_spec_strings=requested_spec_strings, - editable=self.options.editable_install, - namespace_override=self.options.namespace) # TODO: build requirement_specs from requested_collection_specs strings - rc = install.install_requirements_loop(galaxy_context, - requirements_list, - display_callback=self.display, - ignore_errors=self.options.ignore_errors, - no_deps=self.options.no_deps, - force_overwrite=self.options.force) + rc = install.run(galaxy_context, + requirement_spec_strings=self.args, + display_callback=self.display, + ignore_errors=self.options.ignore_errors, + no_deps=self.options.no_deps, + force_overwrite=self.options.force) return rc diff --git a/tests/ansible_galaxy/actions/test_install_action.py b/tests/ansible_galaxy/actions/test_install_action.py index 691de873..0d25587c 100644 --- a/tests/ansible_galaxy/actions/test_install_action.py +++ b/tests/ansible_galaxy/actions/test_install_action.py @@ -87,7 +87,11 @@ def stub_install_repository(*args, **kwargs): for call in mock_ir.call_args_list: log.debug('call: %s', pprint.pformat(list(call))) - assert res == 0 # duh, it's hardcoded + assert isinstance(res, dict) + assert isinstance(res['errors'], list) + assert isinstance(res['success'], bool) + + assert res['success'] is True def test_install_requirements(galaxy_context, mocker): From 66481ba4c86f07eb109910098f8362aa53135d03 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Thu, 23 May 2019 12:59:28 -0400 Subject: [PATCH 13/17] [WIP] install action unit tests --- ansible_galaxy/actions/install.py | 18 +- ansible_galaxy/install.py | 41 ----- .../actions/test_install_action.py | 171 +++++++++++++++++- tests/ansible_galaxy/test_install.py | 50 ++--- 4 files changed, 205 insertions(+), 75 deletions(-) diff --git a/ansible_galaxy/actions/install.py b/ansible_galaxy/actions/install.py index 230ddcc5..f967c29c 100644 --- a/ansible_galaxy/actions/install.py +++ b/ansible_galaxy/actions/install.py @@ -101,7 +101,7 @@ def install_repository(galaxy_context, # See if we can find metadata and/or download the archive before we try to # remove an installed version... try: - find_results = install.find(fetcher) + find_results = fetcher.find() except exceptions.GalaxyError as e: log.debug('requirement_to_install %s failed to be met: %s', requirement_to_install, e) log.warning('Unable to find metadata for %s: %s', requirement_spec_to_install.label, e) @@ -139,19 +139,25 @@ def install_repository(galaxy_context, level='warning') # FETCH state + # Note: fetcher.fetch() as a side effect sets fetcher._archive_path to where it was downloaded to. + try: - fetch_results = install.fetch(fetcher, - repository_spec=repository_spec_to_install, - find_results=find_results) + fetch_results = fetcher.fetch(find_results=find_results) + log.debug('fetch_results: %s', fetch_results) + # fetch_results will include a 'archive_path' pointing to where the artifact # was saved to locally. except exceptions.GalaxyError as e: # fetch error probably should just go to a FAILED state, at least until # we have to implement retries - log.warning('Unable to fetch %s: %s', repository_spec_to_install.name, e) + + log.warning('Unable to fetch %s: %s', + repository_spec_to_install.name, + e) + raise_without_ignore(ignore_errors, e) - # continue + # FIXME: raise ? return None diff --git a/ansible_galaxy/install.py b/ansible_galaxy/install.py index ffd5e4e2..48de583a 100644 --- a/ansible_galaxy/install.py +++ b/ansible_galaxy/install.py @@ -18,47 +18,6 @@ # See actions.install.install_collection for a sketch of the states -def find(fetcher): - """find/discover info about the content""" - - find_results = fetcher.find() - - return find_results - - -# def fetch(fetcher, collection): -# pass - -def fetch(fetcher, repository_spec, find_results): - """download the archive and side effect set self._archive_path to where it was downloaded to. - - MUST be called after self.find().""" - - log.debug('Fetching repository_spec=%s', repository_spec) - - try: - # FIXME: note that ignore_certs for the galaxy - # server(galaxy_context.server['ignore_certs']) - # does not really imply that the repo archive download should ignore certs as well - # (galaxy api server vs cdn) but for now, we use the value for both - fetch_results = fetcher.fetch(find_results=find_results) - except exceptions.GalaxyDownloadError as e: - log.exception(e) - - # TODO: having to keep fetcher state for tracking fetcher.remote_resource and/or cleanup - # is kind of annoying.These methods may need to be in a class. Or maybe - # the GalaxyDownloadError shoud/could have any info. - blurb = 'Failed to fetch the content archive "%s": %s' - log.error(blurb, fetcher.remote_resource, e) - - # reraise, currently handled in main - # TODO: if we support more than one archive per invocation, will need to accumulate errors - # if we want to skip some of them - raise - - return fetch_results - - def repository_spec_from_find_results(find_results, requirement_spec): '''Create a new RepositorySpec with updated info from fetch_results. diff --git a/tests/ansible_galaxy/actions/test_install_action.py b/tests/ansible_galaxy/actions/test_install_action.py index 0d25587c..868754cd 100644 --- a/tests/ansible_galaxy/actions/test_install_action.py +++ b/tests/ansible_galaxy/actions/test_install_action.py @@ -1,11 +1,14 @@ import logging import mock +import pytest + from ansible_galaxy.actions import install from ansible_galaxy import installed_repository_db from ansible_galaxy import exceptions from ansible_galaxy import repository_spec from ansible_galaxy import requirements +from ansible_galaxy.fetch.base import BaseFetch from ansible_galaxy.models.repository import Repository from ansible_galaxy.models.repository_spec import RepositorySpec from ansible_galaxy.models.requirement import Requirement, RequirementOps @@ -181,6 +184,86 @@ def test_install_repositories(galaxy_context, mocker): assert ret == expected_repos +class FauxFetch(BaseFetch): + def __init__(self, galaxy_context, requirement_spec, + find_results=None, fetch_results=None): + super(BaseFetch, self).__init__() + self.requirement_spec = requirement_spec + + self.find_results = find_results or {} + self.fetch_results = fetch_results or {} + log.debug('init FauxFetch') + + def find(self): + log.debug('fauxfetch.find') + return self.find_results + + def fetch(self, find_results): + log.debug('fauxfetch.fetch find_results=%s', find_results) + return self.fetch_results + + +def test_install_repository_find_error(galaxy_context, mocker): + requirements_to_install = \ + requirements.from_dependencies_dict({'some_namespace.this_requires_some_name': '*'}) + + mock_fetcher = mocker.MagicMock(name='MockFetch') + mock_fetcher.find.side_effect = exceptions.GalaxyError('Faux exception during find') + + def faux_get(galaxy_context, requirement_spec): + log.debug('faux get %s', requirement_spec) + return mock_fetcher + + mocker.patch('ansible_galaxy.actions.install.fetch_factory.get', + new=faux_get) + mocker.patch('ansible_galaxy.actions.install.install.install') + + irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) + + with pytest.raises(exceptions.GalaxyError, match='.*Faux exception during find.*') as exc_info: + install.install_repository(galaxy_context, + irdb, + requirement_to_install=requirements_to_install[0], + display_callback=display_callback) + + log.debug('exc_info: %s %r', exc_info, exc_info) + + +def test_install_repository_fetch_error(galaxy_context, mocker): + requirements_to_install = \ + requirements.from_dependencies_dict({'some_namespace.this_requires_some_name': '*'}) + + find_results = {'content': {'galaxy_namespace': 'some_namespace', + 'repo_name': 'some_name'}, + 'custom': {'repo_data': {}, + 'download_url': 'http://foo.invalid/stuff/blip.tar.gz', + 'repoversion': {'version': '9.3.245'}, + }, + } + + def faux_get(galaxy_context, requirement_spec): + log.debug('faux get %s', requirement_spec) + mock_fetcher = mocker.MagicMock(name='MockFetch') + mock_fetcher.find.return_value = find_results + mock_fetcher.fetch.side_effect = exceptions.GalaxyDownloadError(url='http://foo.invalid/stuff/blip.tar.gz') + return mock_fetcher + + mocker.patch('ansible_galaxy.actions.install.fetch_factory.get', + new=faux_get) + mocker.patch('ansible_galaxy.actions.install.install.install') + + irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) + + with pytest.raises(exceptions.GalaxyError, + match='.*Error downloading .*http://foo.invalid/stuff/blip.tar.gz.*') as exc_info: + install.install_repository(galaxy_context, + irdb, + requirement_to_install=requirements_to_install[0], + display_callback=display_callback) + + log.debug('exc_info: %s %r', exc_info, exc_info) + + def test_install_repository_deprecated(galaxy_context, mocker): requirements_to_install = \ requirements.from_dependencies_dict({'some_namespace.this_requires_some_name': '*'}) @@ -194,9 +277,21 @@ def test_install_repository_deprecated(galaxy_context, mocker): }, } - mocker.patch('ansible_galaxy.actions.install.install.find', - return_value=find_results) - mocker.patch('ansible_galaxy.actions.install.install.fetch') + # def faux_get(galaxy_context, requirement_spec): + # log.debug('faux get %s', requirement_spec) + # return FauxFetch(galaxy_context, requirement_spec, + # find_results=find_results) + + mock_fetcher = mocker.MagicMock(name='MockFetch') + mock_fetcher.find.return_value = find_results + mock_fetcher.fetch.return_value = {} + + def faux_get(galaxy_context, requirement_spec): + log.debug('faux get %s', requirement_spec) + return mock_fetcher + + mocker.patch('ansible_galaxy.actions.install.fetch_factory.get', + new=faux_get) mocker.patch('ansible_galaxy.actions.install.install.install') mock_display_callback = mocker.MagicMock(name='mock_display_callback') @@ -215,6 +310,76 @@ def test_install_repository_deprecated(galaxy_context, mocker): assert expected_display_calls in mock_display_callback.call_args_list +def test_install_repository_install_error(galaxy_context, mocker): + requirements_to_install = \ + requirements.from_dependencies_dict({'some_namespace.this_requires_some_name': '*'}) + + find_results = {'content': {'galaxy_namespace': 'some_namespace', + 'repo_name': 'some_name'}, + 'custom': {'repo_data': {}, + 'download_url': 'http://foo.invalid/stuff/blip.tar.gz', + 'repoversion': {'version': '9.3.245'}, + }, + } + + def faux_get(galaxy_context, requirement_spec): + log.debug('faux get %s', requirement_spec) + mock_fetcher = mocker.MagicMock(name='MockFetch') + mock_fetcher.find.return_value = find_results + mock_fetcher.fetch.return_value = {'stuff': 'whatever'} + return mock_fetcher + + mocker.patch('ansible_galaxy.actions.install.fetch_factory.get', + new=faux_get) + mocker.patch('ansible_galaxy.actions.install.install.install', + side_effect=exceptions.GalaxyClientError('Faux galaxy client error from test')) + + irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) + + with pytest.raises(exceptions.GalaxyError, + match='.*Faux galaxy client error from test.*') as exc_info: + install.install_repository(galaxy_context, + irdb, + requirement_to_install=requirements_to_install[0], + display_callback=display_callback) + + log.debug('exc_info: %s %r', exc_info, exc_info) + + +def test_install_repository_install_empty_results(galaxy_context, mocker): + requirements_to_install = \ + requirements.from_dependencies_dict({'some_namespace.this_requires_some_name': '*'}) + + find_results = {'content': {'galaxy_namespace': 'some_namespace', + 'repo_name': 'some_name'}, + 'custom': {'repo_data': {}, + 'download_url': 'http://foo.invalid/stuff/blip.tar.gz', + 'repoversion': {'version': '9.3.245'}, + }, + } + + def faux_get(galaxy_context, requirement_spec): + log.debug('faux get %s', requirement_spec) + mock_fetcher = mocker.MagicMock(name='MockFetch') + mock_fetcher.find.return_value = find_results + mock_fetcher.fetch.return_value = {'stuff': 'whatever'} + return mock_fetcher + + mocker.patch('ansible_galaxy.actions.install.fetch_factory.get', + new=faux_get) + mocker.patch('ansible_galaxy.actions.install.install.install', + return_value=[]) + + irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) + + ret = install.install_repository(galaxy_context, + irdb, + requirement_to_install=requirements_to_install[0], + display_callback=display_callback) + + log.debug('ret: %s', ret) + + def test_install_repositories_no_deps_required(galaxy_context, mocker): needed_deps = [] diff --git a/tests/ansible_galaxy/test_install.py b/tests/ansible_galaxy/test_install.py index 6f329fff..392718a1 100644 --- a/tests/ansible_galaxy/test_install.py +++ b/tests/ansible_galaxy/test_install.py @@ -63,39 +63,39 @@ def test_install(galaxy_context, mocker): assert galaxy_context.collections_path in res[0].path -def test_find(mocker): - mock_fetcher = mocker.MagicMock(name='MockFetch') - mock_fetcher.find.return_value = {} +# def test_find(mocker): +# mock_fetcher = mocker.MagicMock(name='MockFetch') +# mock_fetcher.find.return_value = {} - res = install.find(mock_fetcher) +# res = install.find(mock_fetcher) - log.debug('res: %s', res) +# log.debug('res: %s', res) - assert res == {} +# assert res == {} -def test_fetch(mocker): - mock_fetcher = mocker.MagicMock(name='MockFetch') - mock_fetcher.fetch.return_value = {} - repo_spec = RepositorySpec(namespace='some_namespace', - name='some_name', - version='86.75.30') - find_results = {} +# def test_fetch(mocker): +# mock_fetcher = mocker.MagicMock(name='MockFetch') +# mock_fetcher.fetch.return_value = {} +# repo_spec = RepositorySpec(namespace='some_namespace', +# name='some_name', +# version='86.75.30') +# find_results = {} - res = install.fetch(mock_fetcher, repo_spec, find_results) +# res = install.fetch(mock_fetcher, repo_spec, find_results) - log.debug('res: %s', res) +# log.debug('res: %s', res) -def test_fetch_download_error(mocker): - mock_fetcher = mocker.MagicMock(name='MockFetch') - mock_fetcher.fetch.side_effect = exceptions.GalaxyDownloadError(url='http://example.invalid') +# def test_fetch_download_error(mocker): +# mock_fetcher = mocker.MagicMock(name='MockFetch') +# mock_fetcher.fetch.side_effect = exceptions.GalaxyDownloadError(url='http://example.invalid') - repo_spec = RepositorySpec(namespace='some_namespace', - name='some_name', - version='86.75.30') - find_results = {} +# repo_spec = RepositorySpec(namespace='some_namespace', +# name='some_name', +# version='86.75.30') +# find_results = {} - with pytest.raises(exceptions.GalaxyDownloadError) as exc_info: - install.fetch(mock_fetcher, repo_spec, find_results) - log.debug("exc_info: %s", exc_info) +# with pytest.raises(exceptions.GalaxyDownloadError) as exc_info: +# install.fetch(mock_fetcher, repo_spec, find_results) +# log.debug("exc_info: %s", exc_info) From c4350b24eb9b96fa51f079bfb34ae7fdeb43465f Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Thu, 23 May 2019 13:26:56 -0400 Subject: [PATCH 14/17] Slightly improve install warning if nothing installed --- ansible_galaxy/actions/install.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ansible_galaxy/actions/install.py b/ansible_galaxy/actions/install.py index f967c29c..ea9e00a7 100644 --- a/ansible_galaxy/actions/install.py +++ b/ansible_galaxy/actions/install.py @@ -21,6 +21,7 @@ def raise_without_ignore(ignore_errors, msg=None, rc=1): option --ignore-errors was specified """ ignore_error_blurb = '- you can use --ignore-errors to skip failed collections and finish processing the list.' + # TODO: error if ignore_errors, warn otherwise? if not ignore_errors: # Note: msg may actually be an exception instance or a text string @@ -179,6 +180,7 @@ def install_repository(galaxy_context, force_overwrite=force_overwrite, display_callback=display_callback) except exceptions.GalaxyError as e: + # TODO: make the display an error here? depending on ignore_error? msg = "- %s was NOT installed successfully: %s " display_callback(msg % (found_repository_spec.label, e), level='warning') log.warning(msg, found_repository_spec.label, str(e)) @@ -186,8 +188,10 @@ def install_repository(galaxy_context, return [] if not installed_repositories: - log.warning("- %s was NOT installed successfully.", found_repository_spec.label) - raise_without_ignore(ignore_errors) + msg_tmpl = "- %s was NOT installed successfully:" + log.warning(msg_tmpl, found_repository_spec.label) + msg = msg_tmpl % found_repository_spec.label + raise_without_ignore(ignore_errors, msg) return installed_repositories From fda5f7e6355433f8a06b3eaf7c8fbba51aa1ce69 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Thu, 23 May 2019 13:27:25 -0400 Subject: [PATCH 15/17] Even more install action unit tests --- .../actions/test_install_action.py | 135 ++++++++++-------- tests/ansible_galaxy/test_install.py | 38 ----- 2 files changed, 74 insertions(+), 99 deletions(-) diff --git a/tests/ansible_galaxy/actions/test_install_action.py b/tests/ansible_galaxy/actions/test_install_action.py index 868754cd..05705ee9 100644 --- a/tests/ansible_galaxy/actions/test_install_action.py +++ b/tests/ansible_galaxy/actions/test_install_action.py @@ -1,5 +1,6 @@ import logging import mock +import os import pytest @@ -8,7 +9,6 @@ from ansible_galaxy import exceptions from ansible_galaxy import repository_spec from ansible_galaxy import requirements -from ansible_galaxy.fetch.base import BaseFetch from ansible_galaxy.models.repository import Repository from ansible_galaxy.models.repository_spec import RepositorySpec from ansible_galaxy.models.requirement import Requirement, RequirementOps @@ -18,7 +18,44 @@ def display_callback(msg, **kwargs): - log.debug(msg) + log.debug('%s: kwargs: %r', msg, kwargs) + + +def test_run(galaxy_context, mocker): + mock_results = {'success': True, + 'errors': []} + mocker.patch('ansible_galaxy.actions.install.install_requirements_loop', + return_value=mock_results) + res = install.run(galaxy_context, + requirement_spec_strings=['some_namespace.some_name'], + display_callback=display_callback) + + log.debug('res: %s', res) + assert res == os.EX_OK # 0 + + +def test_run_errors(galaxy_context, mocker): + mock_results = {'success': False, + 'errors': ['The first thing that failed was everything.', + 'Then the rest failed']} + mocker.patch('ansible_galaxy.actions.install.install_requirements_loop', + return_value=mock_results) + + mock_display_callback = mocker.MagicMock(name='mock_display_callback', + wraps=display_callback) + + res = install.run(galaxy_context, + requirement_spec_strings=['some_namespace.some_name'], + display_callback=mock_display_callback) + + log.debug('res: %s', res) + assert res == os.EX_SOFTWARE # 70 + + expected_display_calls = [mocker.call('The first thing that failed was everything.'), + mocker.call('Then the rest failed')] + + log.debug('mdc.call_args_list: %s', mock_display_callback.call_args_list) + assert expected_display_calls in mock_display_callback.call_args_list def test_install_repository_specs_loop(galaxy_context, mocker): @@ -160,58 +197,14 @@ def test_install_repos_empty_requirements(galaxy_context): assert ret == [] -def test_install_repositories(galaxy_context, mocker): - repo_spec = RepositorySpec(namespace='some_namespace', name='some_name', - version='9.4.5') - expected_repos = [Repository(repository_spec=repo_spec)] - - requirements_to_install = \ - requirements.from_dependencies_dict({'some_namespace.this_requires_some_name': '*'}) - - mocker.patch('ansible_galaxy.actions.install.install_repository', - return_value=expected_repos) - - irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) - - ret = install.install_repositories(galaxy_context, - irdb, - requirements_to_install=requirements_to_install, - display_callback=display_callback) - - log.debug('ret: %s', ret) - - assert isinstance(ret, list) - assert ret == expected_repos - - -class FauxFetch(BaseFetch): - def __init__(self, galaxy_context, requirement_spec, - find_results=None, fetch_results=None): - super(BaseFetch, self).__init__() - self.requirement_spec = requirement_spec - - self.find_results = find_results or {} - self.fetch_results = fetch_results or {} - log.debug('init FauxFetch') - - def find(self): - log.debug('fauxfetch.find') - return self.find_results - - def fetch(self, find_results): - log.debug('fauxfetch.fetch find_results=%s', find_results) - return self.fetch_results - - def test_install_repository_find_error(galaxy_context, mocker): requirements_to_install = \ requirements.from_dependencies_dict({'some_namespace.this_requires_some_name': '*'}) - mock_fetcher = mocker.MagicMock(name='MockFetch') - mock_fetcher.find.side_effect = exceptions.GalaxyError('Faux exception during find') - def faux_get(galaxy_context, requirement_spec): log.debug('faux get %s', requirement_spec) + mock_fetcher = mocker.MagicMock(name='MockFetch') + mock_fetcher.find.side_effect = exceptions.GalaxyError('Faux exception during find') return mock_fetcher mocker.patch('ansible_galaxy.actions.install.fetch_factory.get', @@ -277,17 +270,11 @@ def test_install_repository_deprecated(galaxy_context, mocker): }, } - # def faux_get(galaxy_context, requirement_spec): - # log.debug('faux get %s', requirement_spec) - # return FauxFetch(galaxy_context, requirement_spec, - # find_results=find_results) - - mock_fetcher = mocker.MagicMock(name='MockFetch') - mock_fetcher.find.return_value = find_results - mock_fetcher.fetch.return_value = {} - def faux_get(galaxy_context, requirement_spec): log.debug('faux get %s', requirement_spec) + mock_fetcher = mocker.MagicMock(name='MockFetch') + mock_fetcher.find.return_value = find_results + mock_fetcher.fetch.return_value = {} return mock_fetcher mocker.patch('ansible_galaxy.actions.install.fetch_factory.get', @@ -372,13 +359,39 @@ def faux_get(galaxy_context, requirement_spec): irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) - ret = install.install_repository(galaxy_context, - irdb, - requirement_to_install=requirements_to_install[0], - display_callback=display_callback) + with pytest.raises(exceptions.GalaxyError, + match='.*some_namespace.this_requires_some_name was NOT installed successfully.*') as exc_info: + install.install_repository(galaxy_context, + irdb, + requirement_to_install=requirements_to_install[0], + display_callback=display_callback) + + log.debug('exc_info: %s %r', exc_info, exc_info) + + +def test_install_repositories(galaxy_context, mocker): + repo_spec = RepositorySpec(namespace='some_namespace', name='some_name', + version='9.4.5') + expected_repos = [Repository(repository_spec=repo_spec)] + + requirements_to_install = \ + requirements.from_dependencies_dict({'some_namespace.this_requires_some_name': '*'}) + + mocker.patch('ansible_galaxy.actions.install.install_repository', + return_value=expected_repos) + + irdb = installed_repository_db.InstalledRepositoryDatabase(galaxy_context) + + ret = install.install_repositories(galaxy_context, + irdb, + requirements_to_install=requirements_to_install, + display_callback=display_callback) log.debug('ret: %s', ret) + assert isinstance(ret, list) + assert ret == expected_repos + def test_install_repositories_no_deps_required(galaxy_context, mocker): needed_deps = [] diff --git a/tests/ansible_galaxy/test_install.py b/tests/ansible_galaxy/test_install.py index 392718a1..4fefbce7 100644 --- a/tests/ansible_galaxy/test_install.py +++ b/tests/ansible_galaxy/test_install.py @@ -61,41 +61,3 @@ def test_install(galaxy_context, mocker): assert isinstance(res[0], Repository) assert res[0].repository_spec == repo_spec assert galaxy_context.collections_path in res[0].path - - -# def test_find(mocker): -# mock_fetcher = mocker.MagicMock(name='MockFetch') -# mock_fetcher.find.return_value = {} - -# res = install.find(mock_fetcher) - -# log.debug('res: %s', res) - -# assert res == {} - - -# def test_fetch(mocker): -# mock_fetcher = mocker.MagicMock(name='MockFetch') -# mock_fetcher.fetch.return_value = {} -# repo_spec = RepositorySpec(namespace='some_namespace', -# name='some_name', -# version='86.75.30') -# find_results = {} - -# res = install.fetch(mock_fetcher, repo_spec, find_results) - -# log.debug('res: %s', res) - - -# def test_fetch_download_error(mocker): -# mock_fetcher = mocker.MagicMock(name='MockFetch') -# mock_fetcher.fetch.side_effect = exceptions.GalaxyDownloadError(url='http://example.invalid') - -# repo_spec = RepositorySpec(namespace='some_namespace', -# name='some_name', -# version='86.75.30') -# find_results = {} - -# with pytest.raises(exceptions.GalaxyDownloadError) as exc_info: -# install.fetch(mock_fetcher, repo_spec, find_results) -# log.debug("exc_info: %s", exc_info) From 6c5fb7876b4ac296fe55fc7c838e06a3cfab40d4 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Thu, 23 May 2019 13:45:44 -0400 Subject: [PATCH 16/17] Fix flake8 issue in test_requirements.py --- tests/ansible_galaxy/test_requirements.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/ansible_galaxy/test_requirements.py b/tests/ansible_galaxy/test_requirements.py index 84c108ca..a430940c 100644 --- a/tests/ansible_galaxy/test_requirements.py +++ b/tests/ansible_galaxy/test_requirements.py @@ -72,4 +72,3 @@ def test_requirements_from_strings(): assert res[2].requirement_spec.namespace == 'alikins' assert res[2].requirement_spec.name == 'picky' assert res[2].requirement_spec.version_spec == Spec('==3.4.5') - From 51b4eaac12f4d9e541b631f2c20bad4509c8c1ca Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Thu, 23 May 2019 14:06:31 -0400 Subject: [PATCH 17/17] Move repo_spec_from_find_results to repository_spec Out of ansible_galaxy.install and into repository_spec so it is easier to share. --- ansible_galaxy/actions/install.py | 5 ++-- ansible_galaxy/install.py | 37 +----------------------------- ansible_galaxy/repository_spec.py | 38 ++++++++++++++++++++++++++++++- 3 files changed, 41 insertions(+), 39 deletions(-) diff --git a/ansible_galaxy/actions/install.py b/ansible_galaxy/actions/install.py index ea9e00a7..478ffd2b 100644 --- a/ansible_galaxy/actions/install.py +++ b/ansible_galaxy/actions/install.py @@ -8,6 +8,7 @@ from ansible_galaxy import installed_repository_db from ansible_galaxy import matchers from ansible_galaxy import requirements +from ansible_galaxy import repository_spec from ansible_galaxy.fetch import fetch_factory log = logging.getLogger(__name__) @@ -127,8 +128,8 @@ def install_repository(galaxy_context, # TODO: build a new repository_spec based on what we actually fetched to feed to # install etc. The fetcher.fetch() could return a datastructure needed to build # the new one instead of doing it in verify() - found_repository_spec = install.repository_spec_from_find_results(find_results, - requirement_spec_to_install) + found_repository_spec = repository_spec.repository_spec_from_find_results(find_results, + requirement_spec_to_install) log.debug('found_repository_spec: %s', found_repository_spec) diff --git a/ansible_galaxy/install.py b/ansible_galaxy/install.py index 48de583a..f0582b3c 100644 --- a/ansible_galaxy/install.py +++ b/ansible_galaxy/install.py @@ -10,7 +10,7 @@ from ansible_galaxy import exceptions from ansible_galaxy import installed_repository_db from ansible_galaxy.models.install_destination import InstallDestinationInfo -from ansible_galaxy.models.repository_spec import FetchMethods, RepositorySpec +from ansible_galaxy.models.repository_spec import FetchMethods log = logging.getLogger(__name__) @@ -18,41 +18,6 @@ # See actions.install.install_collection for a sketch of the states -def repository_spec_from_find_results(find_results, - requirement_spec): - '''Create a new RepositorySpec with updated info from fetch_results. - - Evolves repository_spec to match fetch results.''' - - # TODO: do we still need to check the fetched version against the spec version? - # We do, since the unspecific version is None, so fetched versions wont match - # so we need a new repository_spec for install. - # TODO: this is more or less a verify/validate step or state transition - content_data = find_results.get('content', {}) - resolved_version = content_data.get('version') - - log.debug('version_spec "%s" for %s was requested and was resolved to version "%s"', - requirement_spec.version_spec, requirement_spec.label, - resolved_version) - - # In theory, a fetch can return a different namespace/name than the one request. This - # is for things like server side aliases. - resolved_name = content_data.get('fetched_name', requirement_spec.name) - resolved_namespace = content_data.get('content_namespace', requirement_spec.namespace) - - # Build a RepositorySpec based on RequirementSpec and the extra info resolved in find() - spec_data = attr.asdict(requirement_spec) - - del spec_data['version_spec'] - - spec_data['version'] = resolved_version - spec_data['namespace'] = resolved_namespace - spec_data['name'] = resolved_name - - repository_spec = RepositorySpec.from_dict(spec_data) - return repository_spec - - def install(galaxy_context, fetcher, fetch_results, diff --git a/ansible_galaxy/repository_spec.py b/ansible_galaxy/repository_spec.py index 59099171..e59cbe60 100644 --- a/ansible_galaxy/repository_spec.py +++ b/ansible_galaxy/repository_spec.py @@ -1,7 +1,8 @@ import logging -from ansible_galaxy import repository_spec_parse +import attr +from ansible_galaxy import repository_spec_parse from ansible_galaxy.models.repository_spec import RepositorySpec @@ -21,3 +22,38 @@ def repository_spec_from_string(repository_spec_string, namespace_override=None, spec_string=spec_data.get('spec_string'), fetch_method=spec_data.get('fetch_method'), src=spec_data.get('src')) + + +def repository_spec_from_find_results(find_results, + requirement_spec): + '''Create a new RepositorySpec with updated info from fetch_results. + + Evolves repository_spec to match fetch results.''' + + # TODO: do we still need to check the fetched version against the spec version? + # We do, since the unspecific version is None, so fetched versions wont match + # so we need a new repository_spec for install. + # TODO: this is more or less a verify/validate step or state transition + content_data = find_results.get('content', {}) + resolved_version = content_data.get('version') + + log.debug('version_spec "%s" for %s was requested and was resolved to version "%s"', + requirement_spec.version_spec, requirement_spec.label, + resolved_version) + + # In theory, a fetch can return a different namespace/name than the one request. This + # is for things like server side aliases. + resolved_name = content_data.get('fetched_name', requirement_spec.name) + resolved_namespace = content_data.get('content_namespace', requirement_spec.namespace) + + # Build a RepositorySpec based on RequirementSpec and the extra info resolved in find() + spec_data = attr.asdict(requirement_spec) + + del spec_data['version_spec'] + + spec_data['version'] = resolved_version + spec_data['namespace'] = resolved_namespace + spec_data['name'] = resolved_name + + repository_spec = RepositorySpec.from_dict(spec_data) + return repository_spec