From bc60d8ccda7a5a5bf0776c83f76c52663378b59c Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Thu, 21 Jan 2021 06:19:29 +1000 Subject: [PATCH] Galaxy - make versions list consistent across versions (#72932) * Galaxy - make versions list consistent across versions * Fix up unit tests --- .../ansible-galaxy-version-response.yml | 4 +++ lib/ansible/galaxy/api.py | 19 ++++++++++-- lib/ansible/galaxy/collection/__init__.py | 31 +++++++++---------- test/units/galaxy/test_collection_install.py | 14 ++++----- 4 files changed, 40 insertions(+), 28 deletions(-) create mode 100644 changelogs/fragments/ansible-galaxy-version-response.yml diff --git a/changelogs/fragments/ansible-galaxy-version-response.yml b/changelogs/fragments/ansible-galaxy-version-response.yml new file mode 100644 index 00000000000..1094cb3cc8d --- /dev/null +++ b/changelogs/fragments/ansible-galaxy-version-response.yml @@ -0,0 +1,4 @@ +minor_changes: +- >- + ansible-galaxy - Ensure ``get_collection_versions`` returns an empty list when a collection does + not exist for consistency across API versions. diff --git a/lib/ansible/galaxy/api.py b/lib/ansible/galaxy/api.py index 61672069026..2ed4930b252 100644 --- a/lib/ansible/galaxy/api.py +++ b/lib/ansible/galaxy/api.py @@ -751,9 +751,15 @@ class GalaxyAPI: server_cache = self._cache.setdefault(get_cache_id(versions_url), {}) modified_cache = server_cache.setdefault('modified', {}) - modified_date = self.get_collection_metadata(namespace, name).modified_str - cached_modified_date = modified_cache.get('%s.%s' % (namespace, name), None) + try: + modified_date = self.get_collection_metadata(namespace, name).modified_str + except GalaxyError as err: + if err.http_code != 404: + raise + # No collection found, return an empty list to keep things consistent with the various APIs + return [] + cached_modified_date = modified_cache.get('%s.%s' % (namespace, name), None) if cached_modified_date != modified_date: modified_cache['%s.%s' % (namespace, name)] = modified_date if versions_url_info.path in server_cache: @@ -763,7 +769,14 @@ class GalaxyAPI: error_context_msg = 'Error when getting available collection versions for %s.%s from %s (%s)' \ % (namespace, name, self.name, self.api_server) - data = self._call_galaxy(versions_url, error_context_msg=error_context_msg, cache=True) + + try: + data = self._call_galaxy(versions_url, error_context_msg=error_context_msg, cache=True) + except GalaxyError as err: + if err.http_code != 404: + raise + # v3 doesn't raise a 404 so we need to mimick the empty response from APIs that do. + return [] if 'data' in data: # v3 automation-hub is the only known API that uses `data` diff --git a/lib/ansible/galaxy/collection/__init__.py b/lib/ansible/galaxy/collection/__init__.py index 276b606bca0..0d7cfc3f785 100644 --- a/lib/ansible/galaxy/collection/__init__.py +++ b/lib/ansible/galaxy/collection/__init__.py @@ -511,29 +511,26 @@ class CollectionRequirement: galaxy_meta = None for api in apis: - try: - if not (requirement == '*' or requirement.startswith('<') or requirement.startswith('>') or - requirement.startswith('!=')): - # Exact requirement - allow_pre_release = True + if not (requirement == '*' or requirement.startswith('<') or requirement.startswith('>') or + requirement.startswith('!=')): + # Exact requirement + allow_pre_release = True - if requirement.startswith('='): - requirement = requirement.lstrip('=') + if requirement.startswith('='): + requirement = requirement.lstrip('=') + try: resp = api.get_collection_version_metadata(namespace, name, requirement) - + except GalaxyError as err: + if err.http_code != 404: + raise + versions = [] + else: galaxy_meta = resp versions = [resp.version] - else: - versions = api.get_collection_versions(namespace, name) - except GalaxyError as err: - if err.http_code != 404: - raise - - versions = [] + else: + versions = api.get_collection_versions(namespace, name) - # Automation Hub doesn't return a 404 but an empty version list so we check that to align both AH and - # Galaxy when the collection is not available on that server. if not versions: display.vvv("Collection '%s' is not available from server %s %s" % (collection, api.name, api.api_server)) diff --git a/test/units/galaxy/test_collection_install.py b/test/units/galaxy/test_collection_install.py index 629a3564fa4..9acf55fd52a 100644 --- a/test/units/galaxy/test_collection_install.py +++ b/test/units/galaxy/test_collection_install.py @@ -403,10 +403,9 @@ def test_build_requirement_from_name_second_server(galaxy_server, monkeypatch): broken_server = copy.copy(galaxy_server) broken_server.api_server = 'https://broken.com/' - mock_404 = MagicMock() - mock_404.side_effect = api.GalaxyError(urllib_error.HTTPError('https://galaxy.server.com', 404, 'msg', {}, - StringIO()), "custom msg") - monkeypatch.setattr(broken_server, 'get_collection_versions', mock_404) + mock_version_list = MagicMock() + mock_version_list.return_value = [] + monkeypatch.setattr(broken_server, 'get_collection_versions', mock_version_list) actual = collection.CollectionRequirement.from_name('namespace.collection', [broken_server, galaxy_server], '>1.0.1', False, True) @@ -420,8 +419,8 @@ def test_build_requirement_from_name_second_server(galaxy_server, monkeypatch): assert actual.latest_version == u'1.0.3' assert actual.dependencies == {} - assert mock_404.call_count == 1 - assert mock_404.mock_calls[0][1] == ('namespace', 'collection') + assert mock_version_list.call_count == 1 + assert mock_version_list.mock_calls[0][1] == ('namespace', 'collection') assert mock_get_versions.call_count == 1 assert mock_get_versions.mock_calls[0][1] == ('namespace', 'collection') @@ -429,8 +428,7 @@ def test_build_requirement_from_name_second_server(galaxy_server, monkeypatch): def test_build_requirement_from_name_missing(galaxy_server, monkeypatch): mock_open = MagicMock() - mock_open.side_effect = api.GalaxyError(urllib_error.HTTPError('https://galaxy.server.com', 404, 'msg', {}, - StringIO()), "") + mock_open.return_value = [] monkeypatch.setattr(galaxy_server, 'get_collection_versions', mock_open)