[Core] az extension add: Improve feedback shown to users when installation is unsuccessful (#22941)

kdestin · web-flow · commit 91af333986b1 · 2022-07-01T22:19:34.000+08:00
* Add more descriptive error messages to resolve_from_index

* Present more feedback to user when `az extension add` or `az extension add --upgrade` fail

* Add/update unittests for resolve_from_index

* Resolve linter errors
diff --git a/src/azure-cli-core/azure/cli/core/extension/_resolve.py b/src/azure-cli-core/azure/cli/core/extension/_resolve.py
@@ -3,6 +3,7 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
 from packaging.version import parse
+from typing import Callable, List, NamedTuple, Union
 
 from azure.cli.core.extension import ext_compat_with_cli, WHEEL_INFO_RE
 from azure.cli.core.extension._index import get_index_extensions
@@ -17,6 +18,13 @@ class NoExtensionCandidatesError(Exception):
     pass
 
 
+class _ExtensionFilter(NamedTuple):
+    # A function that filters a list of extensions
+    filter: Callable[[List[dict]], List[dict]]
+    # Message of exception raised if a filter leaves no candidates
+    on_empty_results_message: Union[str, Callable[[List[dict]], str]]
+
+
 def _is_not_platform_specific(item):
     parsed_filename = WHEEL_INFO_RE(item['filename'])
     p = parsed_filename.groupdict()
@@ -55,38 +63,94 @@ def filter_func(item):
     return filter_func
 
 
-def resolve_from_index(extension_name, cur_version=None, index_url=None, target_version=None, cli_ctx=None):
-    """
-    Gets the download Url and digest for the matching extension
+def _get_latest_version(candidates: List[dict]) -> List[dict]:
+    return [max(candidates, key=lambda c: parse(c['metadata']['version']))]
+
+
+def _get_version_compatibility_feedback(candidates: List[dict]) -> str:
+    from .operations import check_version_compatibility
+
+    try:
+        check_version_compatibility(_get_latest_version(candidates)[0].get("metadata"))
+        return ""
+    except CLIError as e:
+        return e.args[0]
+
 
-    :param cur_version: threshold verssion to filter out extensions.
+def resolve_from_index(extension_name, cur_version=None, index_url=None, target_version=None, cli_ctx=None):
+    """Gets the download Url and digest for the matching extension
+
+    Args:
+        extension_name (str): Name of
+        cur_version (str, optional): Threshold version to filter out extensions. Defaults to None.
+        index_url (str, optional):  Defaults to None.
+        target_version (str, optional): Version of extension to install. Defaults to latest version.
+        cli_ctx (, optional): CLI Context. Defaults to None.
+
+    Raises:
+        NoExtensionCandidatesError when an extension:
+            * Doesn't exist
+            * Has no versions compatible with the current platform
+            * Has no versions more recent than currently installed version
+            * Has no versions that are compatible with the version of azure cli
+
+    Returns:
+        tuple: (Download Url, SHA digest)
     """
     candidates = get_index_extensions(index_url=index_url, cli_ctx=cli_ctx).get(extension_name, [])
 
     if not candidates:
-        raise NoExtensionCandidatesError("No extension found with name '{}'".format(extension_name))
-
-    filters = [_is_not_platform_specific, _is_compatible_with_cli_version]
-    if not target_version:
-        filters.append(_is_greater_than_cur_version(cur_version))
+        raise NoExtensionCandidatesError(f"No extension found with name '{extension_name}'")
 
-    for f in filters:
-        logger.debug("Candidates %s", [c['filename'] for c in candidates])
-        candidates = list(filter(f, candidates))
-    if not candidates:
-        raise NoExtensionCandidatesError("No suitable extensions found.")
+    # Helper to curry predicate functions
+    def list_filter(f):
+        return lambda cs: list(filter(f, cs))
 
-    candidates_sorted = sorted(candidates, key=lambda c: parse(c['metadata']['version']), reverse=True)
-    logger.debug("Candidates %s", [c['filename'] for c in candidates_sorted])
+    candidate_filters = [
+        _ExtensionFilter(
+            filter=list_filter(_is_not_platform_specific),
+            on_empty_results_message=f"No suitable extensions found for '{extension_name}'."
+        )
+    ]
 
     if target_version:
-        try:
-            chosen = [c for c in candidates_sorted if c['metadata']['version'] == target_version][0]
-        except IndexError:
-            raise NoExtensionCandidatesError('Extension with version {} not found'.format(target_version))
+        candidate_filters += [
+            _ExtensionFilter(
+                filter=list_filter(lambda c: c['metadata']['version'] == target_version),
+                on_empty_results_message=f"Version '{target_version}' not found for extension '{extension_name}'"
+            )
+        ]
     else:
-        logger.debug("Choosing the latest of the remaining candidates.")
-        chosen = candidates_sorted[0]
+        candidate_filters += [
+            _ExtensionFilter(
+                filter=list_filter(_is_greater_than_cur_version(cur_version)),
+                on_empty_results_message=f"Latest version of '{extension_name}' is already installed."
+            )
+        ]
+
+    candidate_filters += [
+        _ExtensionFilter(
+            filter=list_filter(_is_compatible_with_cli_version),
+            on_empty_results_message=_get_version_compatibility_feedback
+        ),
+        _ExtensionFilter(
+            filter=_get_latest_version,
+            on_empty_results_message=f"No suitable extensions found for '{extension_name}'."
+        )
+    ]
+
+    for candidate_filter, on_empty_results_message in candidate_filters:
+        logger.debug("Candidates %s", [c['filename'] for c in candidates])
+        filtered_candidates = candidate_filter(candidates)
+
+        if not filtered_candidates and (on_empty_results_message is not None):
+            if not isinstance(on_empty_results_message, str):
+                on_empty_results_message = on_empty_results_message(candidates)
+            raise NoExtensionCandidatesError(on_empty_results_message)
+
+        candidates = filtered_candidates
+
+    chosen = candidates[0]
 
     logger.debug("Chosen %s", chosen)
     download_url, digest = chosen.get('downloadUrl'), chosen.get('sha256Digest')
diff --git a/src/azure-cli-core/azure/cli/core/extension/operations.py b/src/azure-cli-core/azure/cli/core/extension/operations.py
@@ -330,11 +330,7 @@ def add_extension(cmd=None, source=None, extension_name=None, index_url=None, ye
             source, ext_sha256 = resolve_from_index(extension_name, index_url=index_url, target_version=version, cli_ctx=cmd_cli_ctx)
         except NoExtensionCandidatesError as err:
             logger.debug(err)
-
-            if version:
-                err = "No matching extensions for '{} ({})'. Use --debug for more information.".format(extension_name, version)
-            else:
-                err = "No matching extensions for '{}'. Use --debug for more information.".format(extension_name)
+            err = "{}\n\nUse --debug for more information".format(err.args[0])
             raise CLIError(err)
     ext_name, ext_version = _get_extension_info_from_source(source)
     set_extension_management_detail(extension_name if extension_name else ext_name, ext_version)
@@ -397,7 +393,7 @@ def update_extension(cmd=None, extension_name=None, index_url=None, pip_extra_in
             set_extension_management_detail(extension_name, ext_version)
         except NoExtensionCandidatesError as err:
             logger.debug(err)
-            msg = "Extension {} with version {} not found.".format(extension_name, version) if version else "No updates available for '{}'. Use --debug for more information.".format(extension_name)
+            msg = "{}\n\nUse --debug for more information".format(err.args[0])
             logger.warning(msg)
             return
         # Copy current version of extension to tmp directory in case we need to restore it after a failed install.
diff --git a/src/azure-cli-core/azure/cli/core/extension/tests/latest/__init__.py b/src/azure-cli-core/azure/cli/core/extension/tests/latest/__init__.py
@@ -8,6 +8,7 @@
 import tempfile
 import shutil
 from unittest import mock
+from azure.cli.core.extension import EXT_METADATA_MAXCLICOREVERSION, EXT_METADATA_MINCLICOREVERSION
 
 
 def get_test_data_file(filename):
@@ -35,18 +36,21 @@ def __exit__(self, exc_type, exc_val, exc_tb):
         self.patcher.stop()
 
 
-def mock_ext(filename, version=None, download_url=None, digest=None, project_url=None):
+def mock_ext(filename, version=None, download_url=None, digest=None, project_url=None, name=None, min_cli_version=None, max_cli_version=None):
     d = {
         'filename': filename,
         'metadata': {
+            'name': name,
             'version': version,
             'extensions': {
                 'python.details': {
                     'project_urls': {
                         'Home': project_url or 'https://github.com/azure/some-extension'
                     }
                 }
-            }
+            },
+            EXT_METADATA_MINCLICOREVERSION: min_cli_version,
+            EXT_METADATA_MAXCLICOREVERSION: max_cli_version,
         },
         'downloadUrl': download_url or 'http://contoso.com/{}'.format(filename),
         'sha256Digest': digest
diff --git a/src/azure-cli-core/azure/cli/core/extension/tests/latest/test_resolve.py b/src/azure-cli-core/azure/cli/core/extension/tests/latest/test_resolve.py
@@ -3,7 +3,7 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
 import unittest
-
+from unittest import mock
 from azure.cli.core.extension._resolve import (resolve_from_index, resolve_project_url_from_index,
                                                NoExtensionCandidatesError, _is_not_platform_specific,
                                                _is_greater_than_cur_version)
@@ -16,13 +16,13 @@ def test_no_exts_in_index(self):
         name = 'myext'
         with IndexPatch({}), self.assertRaises(NoExtensionCandidatesError) as err:
             resolve_from_index(name)
-        self.assertEqual(str(err.exception), "No extension found with name '{}'".format(name))
+        self.assertEqual(str(err.exception), "No matching extensions for '{}'.".format(name))
 
     def test_ext_not_in_index(self):
         name = 'an_extension_b'
         with IndexPatch({'an_extension_a': []}), self.assertRaises(NoExtensionCandidatesError) as err:
             resolve_from_index(name)
-        self.assertEqual(str(err.exception), "No extension found with name '{}'".format(name))
+        self.assertEqual(str(err.exception), "No matching extensions for '{}'.".format(name))
 
     def test_ext_resolved(self):
         name = 'myext'
@@ -70,9 +70,74 @@ def test_filter_target_version(self):
             self.assertEqual(resolve_from_index(ext_name, target_version='0.2.0')[0], index_data[ext_name][1]['downloadUrl'])
 
         with IndexPatch(index_data):
-            with self.assertRaisesRegex(NoExtensionCandidatesError, 'Extension with version 0.3.0 not found'):
+            with self.assertRaisesRegex(NoExtensionCandidatesError, "Version '0.3.0' not found for extension 'hello'"):
                 resolve_from_index(ext_name, target_version='0.3.0')
 
+    def test_ext_has_available_update(self):
+        ext_name = 'myext'
+        index_data = {
+            ext_name: [
+                mock_ext(f'{ext_name}-0.1.0-py3-none-any.whl', '0.1.0'),
+                mock_ext(f'{ext_name}-0.2.0-py3-none-any.whl', '0.2.0')
+            ]
+        }
+
+        with IndexPatch(index_data):
+            self.assertEqual(resolve_from_index(ext_name, cur_version='0.1.0')[0], index_data[ext_name][1]['downloadUrl'])
+
+
+    def test_ext_has_no_available_updates(self):
+        ext_name = 'myext'
+        index_data = {
+            ext_name: [
+                mock_ext(f'{ext_name}-0.1.0-py3-none-any.whl', '0.1.0'),
+                mock_ext(f'{ext_name}-0.2.0-py3-none-any.whl', '0.2.0')
+            ]
+        }
+
+        with IndexPatch(index_data):
+            with self.assertRaisesRegex(NoExtensionCandidatesError, f"Latest version of '{ext_name}' is already installed."):
+                resolve_from_index(ext_name, cur_version='0.2.0')
+
+    @mock.patch("azure.cli.core.__version__", "2.15.0")
+    def test_ext_requires_later_version_of_cli_core(self):
+        ext_name = 'myext'
+        min_cli_version="2.16.0"
+        index_data = {
+            ext_name: [
+                mock_ext(f'{ext_name}-0.1.0-py3-none-any.whl', '0.1.0', name=ext_name, min_cli_version=min_cli_version),
+                mock_ext(f'{ext_name}-0.2.0-py3-none-any.whl', '0.2.0', name=ext_name, min_cli_version=min_cli_version)
+            ]
+        }
+
+        exception_regex = '\n'.join([
+            f'This extension requires a min of {min_cli_version} CLI core.',
+            "Please run 'az upgrade' to upgrade to a compatible version."
+        ])
+        with IndexPatch(index_data):
+            with self.assertRaisesRegex(NoExtensionCandidatesError, exception_regex):
+                resolve_from_index(ext_name)
+
+
+    @mock.patch("azure.cli.core.__version__", "2.15.0")
+    def test_ext_requires_earlier_version_of_cli_core(self):
+        ext_name = 'myext'
+        max_cli_version="2.14.0"
+        index_data = {
+            ext_name: [
+                mock_ext(f'{ext_name}-0.1.0-py3-none-any.whl', '0.1.0', name=ext_name, max_cli_version=max_cli_version),
+                mock_ext(f'{ext_name}-0.2.0-py3-none-any.whl', '0.2.0', name=ext_name, max_cli_version=max_cli_version)
+            ]
+        }
+        exception_regex = '\n'.join([
+            f'This extension requires a max of {max_cli_version} CLI core.',
+            f"Please run 'az extension update -n {ext_name}' to update the extension."
+        ])
+
+        with IndexPatch(index_data):
+            with self.assertRaisesRegex(NoExtensionCandidatesError, exception_regex):
+                resolve_from_index(ext_name)
+
 
 class TestResolveFilters(unittest.TestCase):
 
