From 7e086d854194bbcffe65fbd2e5d0fcd8bd0bd74b Mon Sep 17 00:00:00 2001 From: houk-ms Date: Wed, 19 Aug 2020 15:34:30 +0800 Subject: [PATCH 01/20] draft for new error schema --- src/azure-cli-core/azure/cli/core/_help.py | 16 + .../azure/cli/core/commands/__init__.py | 19 +- .../azure/cli/core/commands/arm.py | 6 +- src/azure-cli-core/azure/cli/core/parser.py | 307 +++++++++++++++--- .../azure/cli/core/telemetry.py | 23 ++ .../azure/cli/core/tests/test_parser.py | 4 +- .../azure/cli/core/tests/test_util.py | 19 +- src/azure-cli-core/azure/cli/core/util.py | 169 +++++++--- src/azure-cli/azure/cli/__main__.py | 8 +- 9 files changed, 467 insertions(+), 104 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_help.py b/src/azure-cli-core/azure/cli/core/_help.py index 90432397383..46d9c4e7888 100644 --- a/src/azure-cli-core/azure/cli/core/_help.py +++ b/src/azure-cli-core/azure/cli/core/_help.py @@ -183,6 +183,22 @@ def show_help(self, cli_name, nouns, parser, is_group): if not nouns: print(UX_SURVEY_PROMPT_COLOR if self.cli_ctx.enable_color else UX_SURVEY_PROMPT) + def get_examples(self, command, parser, is_group): + nouns = command.split(' ')[1:] + self.update_loaders_with_help_file_contents(nouns) + + delimiters = ' '.join(nouns) + help_file = self.command_help_cls(self, delimiters, parser) if not is_group \ + else self.group_help_cls(self, delimiters, parser) + help_file.load(parser) + + def strip_command(command): + command = command.replace('\\\n', '') + contents = [item for item in command.split(' ') if item] + return ' '.join(contents).strip() + + return [strip_command(example.command) for example in help_file.examples] + def _register_help_loaders(self): import azure.cli.core._help_loaders as help_loaders import inspect diff --git a/src/azure-cli-core/azure/cli/core/commands/__init__.py b/src/azure-cli-core/azure/cli/core/commands/__init__.py index d567c85bc3a..5c063013c29 100644 --- a/src/azure-cli-core/azure/cli/core/commands/__init__.py +++ b/src/azure-cli-core/azure/cli/core/commands/__init__.py @@ -845,20 +845,37 @@ def _validate_cmd_level(self, ns, cmd_validator): # pylint: disable=no-self-use pass def _validate_arg_level(self, ns, **_): # pylint: disable=no-self-use + from azure.cli.core.util import AzCLIErrorType + from azure.cli.core.util import AzCLIError for validator in getattr(ns, '_argument_validators', []): try: validator(**self._build_kwargs(validator, ns)) + except AzCLIError: + raise except Exception as ex: # Delay the import and mimic an exception handler from msrest.exceptions import ValidationError if isinstance(ex, ValidationError): logger.debug('Validation error in %s.', str(validator)) - raise + raise AzCLIError(AzCLIErrorType.ValidationError, getattr(ex, 'message', str(ex))) try: delattr(ns, '_argument_validators') except AttributeError: pass + def _validation(self, parsed_ns): + try: + cmd_validator = getattr(parsed_ns, '_command_validator', None) + if cmd_validator: + self._validate_cmd_level(parsed_ns, cmd_validator) + else: + self._validate_arg_level(parsed_ns) + except CLIError: + raise + except Exception: # pylint: disable=broad-except + err = sys.exc_info()[1] + getattr(parsed_ns, '_parser', self.parser).validation_error(str(err)) + class LongRunningOperation: # pylint: disable=too-few-public-methods def __init__(self, cli_ctx, start_msg='', finish_msg='', poller_done_interval_ms=1000.0): diff --git a/src/azure-cli-core/azure/cli/core/commands/arm.py b/src/azure-cli-core/azure/cli/core/commands/arm.py index 97e0adc06d3..9031401fd94 100644 --- a/src/azure-cli-core/azure/cli/core/commands/arm.py +++ b/src/azure-cli-core/azure/cli/core/commands/arm.py @@ -758,8 +758,12 @@ def show_exception_handler(ex): if getattr(getattr(ex, 'response', ex), 'status_code', None) == 404: import sys from azure.cli.core.azlogging import CommandLoggerContext + from azure.cli.core.util import AzCLIErrorType + from azure.cli.core.util import AzCLIError with CommandLoggerContext(logger): - logger.error(getattr(ex, 'message', ex)) + az_error = AzCLIError(AzCLIErrorType.ValidationError, getattr(ex, 'message', ex)) + az_error.print_error() + az_error.send_telemetry() sys.exit(3) raise ex diff --git a/src/azure-cli-core/azure/cli/core/parser.py b/src/azure-cli-core/azure/cli/core/parser.py index 6e56daa5015..aac956da229 100644 --- a/src/azure-cli-core/azure/cli/core/parser.py +++ b/src/azure-cli-core/azure/cli/core/parser.py @@ -17,6 +17,8 @@ from azure.cli.core.commands import ExtensionCommandSource from azure.cli.core.commands import AzCliCommandInvoker from azure.cli.core.commands.events import EVENT_INVOKER_ON_TAB_COMPLETION +from azure.cli.core.util import AzCLIErrorType +from azure.cli.core.util import AzCLIError from knack.log import get_logger from knack.parser import CLICommandParser @@ -24,6 +26,15 @@ logger = get_logger(__name__) +EXTENSION_REFERENCE = ("If the command is from an extension, " + "please make sure the corresponding extension is installed. " + "To learn more about extensions, please visit " + "'https://docs.microsoft.com/en-us/cli/azure/azure-cli-extensions-overview'") + +OVERVIEW_REFERENCE = ("Still stuck? Run '{command} --help' to view all commands or go to " + "'https://docs.microsoft.com/en-us/cli/azure/reference-index?view=azure-cli-latest' " + "to learn more") + class IncorrectUsageError(CLIError): '''Raised when a command is incorrectly used and the usage should be @@ -50,6 +61,185 @@ def _get_completions(self, comp_words, cword_prefix, cword_prequote, last_wordbr last_wordbreak_pos) +class CommandRecommender(): + """Recommend a command for user when user's command fails. + It combines alladin recommendations and examples in help files.""" + + def __init__(self, command, parameters, extension, cli_ctx): + """ + :param command: The command name in user's input, which should be corrected if misspelled. + :type command: str + :param parameters: The parameter arguments in users input. + :type parameters: list + :param extension: The extension name in user's input if the command comes from an extension. + :type extension: str + :param cli_ctx: CLI context when parser fails. + :type cli_ctx: knack.cli.CLI + """ + self.command = command.strip() + self.extension = extension + self.cli_ctx = cli_ctx + + self.parameters = self._normalize_parameters(parameters) + self.help_examples = [] + self.aladdin_recommendations = [] + + def set_help_examples(self, examples): + """Set recommendations from help files""" + + self.help_examples.extend(examples) + + def _set_aladdin_recommendations(self): + """Set recommendations from aladdin service. + Call the aladdin service API, parse the response and set the recommendations. + """ + + import json + import requests + from requests import RequestException + from http import HTTPStatus + from azure.cli.core import __version__ as version + + api_url = 'https://app.aladdin.microsoft.com/api/v1.0/suggestions' + headers = {'Content-Type': 'application/json'} + correlation_id = telemetry._session.correlation_id # pylint: disable=protected-access + subscription_id = telemetry._get_azure_subscription_id() # pylint: disable=protected-access + + context = { + 'versionNumber': version + } + if telemetry.is_telemetry_enabled(): + if correlation_id: + context['correlationId'] = correlation_id + if subscription_id: + context['subscriptionId'] = subscription_id + + parameters = [item for item in self.parameters if item not in ['--debug', '--verbose']] + query = { + "command": self.command, + "parameters": ','.join(parameters) + } + + response = None + try: + response = requests.get( + api_url, + params={ + 'query': json.dumps(query), + 'clientType': 'AzureCli', + 'context': json.dumps(context) + }, + headers=headers) + except RequestException as ex: + logger.debug('Recommendation requests.get() exception: %s', ex) + + recommendations = [] + if response and response.status_code == HTTPStatus.OK: + for result in response.json(): + # parse the reponse and format the recommendation + command, parameters, placeholders = result['SuccessCommand'],\ + result['SuccessCommand_Parameters'].split(','),\ + result['SuccessCommand_ArgumentPlaceholders'].split('♠') + recommendation = 'az {} '.format(command) + for parameter, placeholder in zip(parameters, placeholders): + recommendation += '{} {} '.format(parameter, placeholder) + recommendations.append(recommendation.strip()) + + self.aladdin_recommendations.extend(recommendations) + + def recommend_a_command(self): + """Recommend a command for user when user's command fails. + The recommended command will be the best matched one from + both the help files and the aladdin recommendations. + """ + self._set_aladdin_recommendations() + # all the recommended commands from help examples and aladdin + all_commands = self.help_examples + self.aladdin_recommendations + all_commands.sort(key=len) + + filtered_commands = [] + filtered_choices = [] + target = ''.join(self.parameters) + + for command in all_commands: + # filter out the commands which begins with a different command group + if command.startswith('az {}'.format(self.command)): + parameters = self._get_parameter_list(command) + normalized_parameters = self._normalize_parameters(parameters) + filtered_choices.append(''.join(normalized_parameters)) + filtered_commands.append(command) + + # sort the commands by argument matches + candidates = difflib.get_close_matches(target, filtered_choices, cutoff=0) + + recommend_command = '' + if candidates: + index = filtered_choices.index(candidates[0]) + recommend_command = filtered_commands[index] + + return recommend_command + + def _get_parameter_list(self, raw_command): # pylint: disable=no-self-use + """Get the paramter list from a raw command string + An example: 'az group create -n test -l eastus' ==> ['-n', '-l'] + """ + contents = raw_command.split(' ') + return [item for item in contents if item.startswith('-')] + + def _normalize_parameters(self, parameters): + """Normalize a parameter list. + Get the standard form of a parameter list, which includes: + 1. Use long options to replace short options + 2. Remove the unrecognized parameters + 3. Sort the result parameter list + An example: ['-g', '-n'] ==> ['--name', '--resource-group'] + """ + + from knack.deprecation import Deprecated + + normalized_parameters = [] + try: + cmd_table = self.cli_ctx.invocation.commands_loader.command_table.get(self.command, None) + parameter_table = cmd_table.arguments if cmd_table else None + except AttributeError: + parameter_table = None + + if parameters: + rules = { + '-h': '--help', + '-o': '--output', + '--only-show-errors': None, + '--help': None, + '--output': None, + '--query': None, + '--debug': None, + '--verbose': None + } + + if parameter_table: + for argument in parameter_table.values(): + options = argument.type.settings['options_list'] + options = (option for option in options if not isinstance(option, Deprecated)) + try: + sorted_options = sorted(options, key=len, reverse=True) + standard_form = sorted_options[0] + + for option in sorted_options[1:]: + rules[option] = standard_form + rules[standard_form] = None + except TypeError: + logger.debug('Unexpected argument options `%s` of type `%s`.', options, type(options).__name__) + + for parameter in parameters: + if parameter in rules: + normalized_form = rules.get(parameter, None) or parameter + normalized_parameters.append(normalized_form) + else: + logger.debug('"%s" is an invalid parameter for command "%s".', parameter, self.command) + + return sorted(normalized_parameters) + + class AzCliCommandParser(CLICommandParser): """ArgumentParser implementation specialized for the Azure CLI utility.""" @@ -143,18 +333,29 @@ def load_command_table(self, command_loader): _parser=command_parser) def validation_error(self, message): - telemetry.set_user_fault('validation error: {}'.format(message)) - return super(AzCliCommandParser, self).error(message) + az_error = AzCLIError(AzCLIErrorType.ValidationError, message, command=self.prog) + az_error.print_error() + az_error.send_telemetry() + self.exit(2) def error(self, message): - telemetry.set_user_fault('parse error: {}'.format(message)) - args = {'prog': self.prog, 'message': message} - with CommandLoggerContext(logger): - logger.error('%(prog)s: error: %(message)s', args) - self.print_usage(sys.stderr) - # Manual recommendations - self._set_manual_recommendations(args['message']) - # AI recommendations + # Get a recommended command from the CommandRecommender + command_arguments = self._get_failure_recovery_arguments() + recommender = CommandRecommender(*command_arguments, self.cli_ctx) + recommender.set_help_examples(self.get_examples(self.prog)) + recommendation = recommender.recommend_a_command() + + az_error = AzCLIError(AzCLIErrorType.ArgumentParseError, message, command=self.prog) + if '--query' in message: + from azure.cli.core.util import QUERY_REFERENCE + az_error.set_recommendation(QUERY_REFERENCE) + elif recommendation: + az_error.set_recommendation("Try this: '{}'".format(recommendation)) + az_error.set_recommendation(OVERVIEW_REFERENCE.format(command=self.prog)) + az_error.print_error() + az_error.send_telemetry() + + # For ai-did-you-mean-this failure_recovery_recommendations = self._get_failure_recovery_recommendations() self._suggestion_msg.extend(failure_recovery_recommendations) self._print_suggestion_msg(sys.stderr) @@ -177,19 +378,19 @@ def format_help(self): telemetry.set_success(summary='show help') super(AzCliCommandParser, self).format_help() + def get_examples(self, command): + if not self.cli_help: + return [] + is_group = self.is_group() + return self.cli_help.get_examples(command, + self._actions[-1] if is_group else self, + is_group) + def enable_autocomplete(self): argcomplete.autocomplete = AzCompletionFinder() argcomplete.autocomplete(self, validator=lambda c, p: c.lower().startswith(p.lower()), default_completer=lambda _: ()) - def _set_manual_recommendations(self, error_msg): - recommendations = [] - # recommendation for --query value error - if '--query' in error_msg: - recommendations.append('To learn more about [--query JMESPATH] usage in AzureCLI, ' - 'visit https://aka.ms/CLIQuery') - self._suggestion_msg.extend(recommendations) - def _get_failure_recovery_arguments(self, action=None): # Strip the leading "az " and any extraneous whitespace. command = self.prog[3:].strip() @@ -349,13 +550,21 @@ def _check_value(self, action, value): # pylint: disable=too-many-statements, t # Override to customize the error message when a argument is not among the available choices # converted value must be one of the choices (if specified) if action.choices is not None and value not in action.choices: # pylint: disable=too-many-nested-blocks + # self.cli_ctx is None when self.prog is beyond 'az', such as 'az iot'. + # use cli_ctx from cli_help which is not lost. + cli_ctx = self.cli_ctx or (self.cli_help.cli_ctx if self.cli_help else None) + caused_by_extension_not_installed = False + command_name_inferred = self.prog if not self.command_source: candidates = difflib.get_close_matches(value, action.choices, cutoff=0.7) + if candidates: + # use the most likely candidate to replace the misspelled command + args = self.prog.split() + self._raw_arguments + args_inferred = [item if item != value else candidates[0] for item in args] + command_name_inferred = ' '.join(args_inferred).split('-')[0] + error_msg = None - # self.cli_ctx is None when self.prog is beyond 'az', such as 'az iot'. - # use cli_ctx from cli_help which is not lost. - cli_ctx = self.cli_ctx or (self.cli_help.cli_ctx if self.cli_help else None) use_dynamic_install = self._get_extension_use_dynamic_install_config() if use_dynamic_install != 'no' and not candidates: # Check if the command is from an extension @@ -400,46 +609,50 @@ def _check_value(self, action, value): # pylint: disable=too-many-statements, t import subprocess import platform exit_code = subprocess.call(cmd_list, shell=platform.system() == 'Windows') - telemetry.set_user_fault("Extension {} dynamically installed and commands will be " - "rerun automatically.".format(ext_name)) self.exit(exit_code) else: - error_msg = 'Extension {} installed. Please rerun your command.'.format(ext_name) + with CommandLoggerContext(logger): + reminder = 'Extension {} installed. Please rerun your command.'.format(ext_name) + logger.error(reminder) + self.exit(2) else: error_msg = "The command requires the extension {ext_name}. " \ "To install, run 'az extension add -n {ext_name}'.".format(ext_name=ext_name) if not error_msg: # parser has no `command_source`, value is part of command itself - error_msg = "{prog}: '{value}' is not in the '{prog}' command group. See '{prog} --help'." \ - .format(prog=self.prog, value=value) - if use_dynamic_install.lower() == 'no': - extensions_link = 'https://docs.microsoft.com/en-us/cli/azure/azure-cli-extensions-overview' - error_msg = ("{msg} " - "If the command is from an extension, " - "please make sure the corresponding extension is installed. " - "To learn more about extensions, please visit " - "{extensions_link}").format(msg=error_msg, extensions_link=extensions_link) + error_msg = "'{value}' is misspelled or not recognized by the system.".format(value=value) + az_error = AzCLIError(AzCLIErrorType.CommandNotFoundError, error_msg, command=self.prog) + else: # `command_source` indicates command values have been parsed, value is an argument parameter = action.option_strings[0] if action.option_strings else action.dest - error_msg = "{prog}: '{value}' is not a valid value for '{param}'. See '{prog} --help'.".format( + error_msg = "{prog}: '{value}' is not a valid value for '{param}'.".format( prog=self.prog, value=value, param=parameter) candidates = difflib.get_close_matches(value, action.choices, cutoff=0.7) + az_error = AzCLIError(AzCLIErrorType.ArgumentParseError, error_msg, command=self.prog) - telemetry.set_user_fault(error_msg) - with CommandLoggerContext(logger): - logger.error(error_msg) - if not caused_by_extension_not_installed: - if candidates: - print_args = { - 's': 's' if len(candidates) > 1 else '', - 'verb': 'are' if len(candidates) > 1 else 'is', - 'value': value - } - self._suggestion_msg.append("\nThe most similar choice{s} to '{value}' {verb}:" - .format(**print_args)) - self._suggestion_msg.append('\n'.join(['\t' + candidate for candidate in candidates])) + _, params, extension = self._get_failure_recovery_arguments() + if candidates: + az_error.set_recommendation("Did you mean '{}' ?".format(candidates[0])) + # recommand a command for user + recommender = CommandRecommender(command_name_inferred[3:].strip(), params, extension, cli_ctx) + recommender.set_help_examples(self.get_examples(command_name_inferred)) + recommended_command = recommender.recommend_a_command() + if recommended_command: + az_error.set_recommendation("Try this: '{}'".format(recommended_command)) + + # remind user to check extensions if we can not find a command to recommend + if az_error.error_type == AzCLIErrorType.CommandNotFoundError \ + and not az_error.recommendations and self.prog == 'az': + az_error.set_recommendation(EXTENSION_REFERENCE) + + az_error.set_recommendation(OVERVIEW_REFERENCE.format(command=self.prog)) + + az_error.print_error() + az_error.send_telemetry() + + if not caused_by_extension_not_installed: failure_recovery_recommendations = self._get_failure_recovery_recommendations(action) self._suggestion_msg.extend(failure_recovery_recommendations) self._print_suggestion_msg(sys.stderr) diff --git a/src/azure-cli-core/azure/cli/core/telemetry.py b/src/azure-cli-core/azure/cli/core/telemetry.py index cc10885e88b..06a6e2de38e 100644 --- a/src/azure-cli-core/azure/cli/core/telemetry.py +++ b/src/azure-cli-core/azure/cli/core/telemetry.py @@ -45,6 +45,12 @@ def __init__(self, correlation_id=None, application=None): self.extension_management_detail = None self.raw_command = None self.mode = 'default' + # The AzCLIErrorType + self.error_type = 'None' + # The class name of the raw exception + self.exception_name = 'None' + # The stacktrace of the raw exception + self.stack_trace = 'None' self.init_time_elapsed = None self.invoke_time_elapsed = None # A dictionary with the application insight instrumentation key @@ -55,6 +61,13 @@ def __init__(self, correlation_id=None, application=None): self.suppress_new_event = False def add_exception(self, exception, fault_type, description=None, message=''): + # Move the exception info into userTask record, in order to make one Telemetry record for one command + self.exception_name = exception.__class__.__name__ + self.result_summary = _remove_cmd_chars(message or str(exception)) + self.stack_trace = _remove_cmd_chars(_get_stack_trace()) + + # Backward compatible, so there are duplicated info recorded + # The logic below should be removed along with self.exceptions after confirmation fault_type = _remove_symbols(fault_type).replace('"', '').replace("'", '').replace(' ', '-') details = { 'Reserved.DataModel.EntityType': 'Fault', @@ -179,6 +192,9 @@ def _get_azure_cli_properties(self): set_custom_properties(result, 'Mode', self.mode) from azure.cli.core._environment import _ENV_AZ_INSTALLER set_custom_properties(result, 'Installer', os.getenv(_ENV_AZ_INSTALLER)) + set_custom_properties(result, 'error_type', self.error_type) + set_custom_properties(result, 'exception_name', self.exception_name) + set_custom_properties(result, 'stack_trace', self.stack_trace) return result @@ -286,6 +302,13 @@ def set_exception(exception, fault_type, summary=None): _session.add_exception(exception, fault_type=fault_type, description=summary) +@decorators.suppress_all_exceptions() +def set_error_type(error_type): + if _session.result != 'None': + return + _session.error_type = error_type + + @decorators.suppress_all_exceptions() def set_failure(summary=None): if _session.result != 'None': diff --git a/src/azure-cli-core/azure/cli/core/tests/test_parser.py b/src/azure-cli-core/azure/cli/core/tests/test_parser.py index 47708a6d397..0507ed2c58f 100644 --- a/src/azure-cli-core/azure/cli/core/tests/test_parser.py +++ b/src/azure-cli-core/azure/cli/core/tests/test_parser.py @@ -239,13 +239,13 @@ def mock_add_extension(*args, **kwargs): # assert the right type of error msg is logged for command vs argument parsing self.assertEqual(len(logger_msgs), 5) for msg in logger_msgs[:3]: - self.assertIn("not in the", msg) - self.assertIn("command group", msg) + self.assertIn("CommandNotFoundError", msg) for msg in logger_msgs[3:]: self.assertIn("not a valid value for '--opt'.", msg) # assert the right choices are matched as "close". # If these don't hold, matching algorithm should be deemed flawed. + choice_lists = [choice_lists[index] for index in range(len(choice_lists)) if not index % 2] for choices in choice_lists[:2]: self.assertEqual(len(choices), 1) self.assertEqual(len(choice_lists[2]), 0) diff --git a/src/azure-cli-core/azure/cli/core/tests/test_util.py b/src/azure-cli-core/azure/cli/core/tests/test_util.py index 20a31fa2685..8d5db93546a 100644 --- a/src/azure-cli-core/azure/cli/core/tests/test_util.py +++ b/src/azure-cli-core/azure/cli/core/tests/test_util.py @@ -401,7 +401,7 @@ def test_handle_exception_keyboardinterrupt(self, mock_logger_error): ex_result = handle_exception(keyboard_interrupt_ex) # test behavior - self.assertFalse(mock_logger_error.called) + self.assertTrue(mock_logger_error.called) self.assertEqual(ex_result, 1) @mock.patch('azure.cli.core.util.logger.error', autospec=True) @@ -417,7 +417,7 @@ def test_handle_exception_clierror(self, mock_logger_error): # test behavior self.assertTrue(mock_logger_error.called) - self.assertEqual(mock.call(err_msg), mock_logger_error.call_args) + self.assertIn(err_msg, mock_logger_error.call_args.args[0]) self.assertEqual(ex_result, 1) @mock.patch('azure.cli.core.util.logger.error', autospec=True) @@ -435,8 +435,8 @@ def test_handle_exception_clouderror(self, mock_logger_error): # test behavior self.assertTrue(mock_logger_error.called) - self.assertEqual(mock.call(mock_cloud_error.args[0]), mock_logger_error.call_args) - self.assertEqual(ex_result, mock_cloud_error.args[1]) + self.assertIn(mock_cloud_error.args[0], mock_logger_error.call_args.args[0]) + self.assertEqual(ex_result, 1) @mock.patch('azure.cli.core.util.logger.error', autospec=True) def test_handle_exception_httpoperationerror_typical_response_error(self, mock_logger_error): @@ -447,14 +447,13 @@ def test_handle_exception_httpoperationerror_typical_response_error(self, mock_l response_text = json.dumps(err) mock_http_error = self._get_mock_HttpOperationError(response_text) - expected_call = mock.call("%s%s", "{} - ".format(err_code), err_msg) - # call handle_exception ex_result = handle_exception(mock_http_error) # test behavior self.assertTrue(mock_logger_error.called) - self.assertEqual(expected_call, mock_logger_error.call_args) + self.assertIn(err_msg, mock_logger_error.call_args.args[0]) + self.assertIn(err_code, mock_logger_error.call_args.args[0]) self.assertEqual(ex_result, 1) @mock.patch('azure.cli.core.util.logger.error', autospec=True) @@ -474,7 +473,7 @@ def test_handle_exception_httpoperationerror_error_key_has_string_value(self, mo # test behavior self.assertTrue(mock_logger_error.called) - self.assertEqual(mock.call(expected_message), mock_logger_error.call_args) + self.assertIn(expected_message, mock_logger_error.call_args.args[0]) self.assertEqual(ex_result, 1) @mock.patch('azure.cli.core.util.logger.error', autospec=True) @@ -492,7 +491,7 @@ def test_handle_exception_httpoperationerror_no_error_key(self, mock_logger_erro # test behavior self.assertTrue(mock_logger_error.called) - self.assertEqual(mock.call(mock_http_error), mock_logger_error.call_args) + self.assertIn(str(mock.call(mock_http_error).args[0]), mock_logger_error.call_args.args[0]) self.assertEqual(ex_result, 1) @mock.patch('azure.cli.core.util.logger.error', autospec=True) @@ -509,7 +508,7 @@ def test_handle_exception_httpoperationerror_no_response_text(self, mock_logger_ # test behavior self.assertTrue(mock_logger_error.called) - self.assertEqual(mock.call(mock_http_error), mock_logger_error.call_args) + self.assertIn(str(mock.call(mock_http_error).args[0]), mock_logger_error.call_args.args[0]) self.assertEqual(ex_result, 1) @staticmethod diff --git a/src/azure-cli-core/azure/cli/core/util.py b/src/azure-cli-core/azure/cli/core/util.py index 141ca271eee..01fb6f145d8 100644 --- a/src/azure-cli-core/azure/cli/core/util.py +++ b/src/azure-cli-core/azure/cli/core/util.py @@ -14,6 +14,7 @@ import ssl import re import logging +from enum import Enum import six from six.moves.urllib.request import urlopen # pylint: disable=import-error @@ -26,10 +27,10 @@ COMPONENT_PREFIX = 'azure-cli-' SSLERROR_TEMPLATE = ('Certificate verification failed. This typically happens when using Azure CLI behind a proxy ' - 'that intercepts traffic with a self-signed certificate. ' - # pylint: disable=line-too-long - 'Please add this certificate to the trusted CA bundle: https://github.com/Azure/azure-cli/blob/dev/doc/use_cli_effectively.md#work-behind-a-proxy. ' - 'Error detail: {}') + 'that intercepts traffic with a self-signed certificate. ') + +QUERY_REFERENCE = ("To learn more about --query, please visit: " + "'https://docs.microsoft.com/cli/azure/query-azure-cli?view=azure-cli-latest'") _PROXYID_RE = re.compile( '(?i)/subscriptions/(?P[^/]*)(/resourceGroups/(?P[^/]*))?' @@ -37,6 +38,7 @@ _CHILDREN_RE = re.compile('(?i)/(?P[^/]*)/(?P[^/]*)') +# pylint: disable=line-too-long _PACKAGE_UPGRADE_INSTRUCTIONS = {"YUM": ("sudo yum update -y azure-cli", "https://aka.ms/doc/UpdateAzureCliYum"), "ZYPPER": ("sudo zypper refresh && sudo zypper update -y azure-cli", "https://aka.ms/doc/UpdateAzureCliZypper"), "DEB": ("sudo apt-get update && sudo apt-get install --only-upgrade -y azure-cli", "https://aka.ms/doc/UpdateAzureCliApt"), @@ -60,7 +62,83 @@ ] -def handle_exception(ex): # pylint: disable=too-many-return-statements +class AzCLIErrorType(Enum): + """ AzureCLI error types """ + + # userfaults + CommandNotFoundError = 'CommandNotFoundError' + ArgumentParseError = 'ArgumentParseError' + ValidationError = 'ValidationError' + ManualInterrupt = 'ManualInterrupt' + # service side error + ServiceError = 'ServiceError' + # client side error + ClientError = 'ClientError' + # unexpected error + UnexpectedError = 'UnexpectedError' + + +class AzCLIError(CLIError): + """ AzureCLI error definition """ + + def __init__(self, error_type, error_msg, raw_exception=None, command=None): + """ + :param error_type: The name of the AzureCLI error type. + :type error_type: azure.cli.core.util.AzCLIErrorType + :param error_msg: The error message detail. + :type error_msg: str + :param raw_exception: The raw exception. + :type raw_exception: Exception + :param command: The command which brings the error. + :type command: str + :param recommendations: The recommendations to resolve the error. + :type recommendations: list + """ + self.error_type = error_type + self.error_msg = error_msg + self.raw_exception = raw_exception + self.command = command + self.recommendations = [] + super().__init__(error_msg) + + def set_recommendation(self, recommendation): + self.recommendations.append(recommendation) + + def set_raw_exception(self, raw_exception): + self.raw_exception = raw_exception + + def print_error(self): + from azure.cli.core.azlogging import CommandLoggerContext + with CommandLoggerContext(logger): + message = '{}: {}'.format(self.error_type.value, self.error_msg) + logger.error(message) + if self.raw_exception: + logger.exception(self.raw_exception) + if self.recommendations: + for recommendation in self.recommendations: + print(recommendation, file=sys.stderr) + + def send_telemetry(self): + import azure.cli.core.telemetry as telemetry + telemetry.set_error_type(self.error_type.value) + + # For userfaults + if self.error_type in [AzCLIErrorType.CommandNotFoundError, + AzCLIErrorType.ArgumentParseError, + AzCLIErrorType.ValidationError, + AzCLIErrorType.ManualInterrupt]: + telemetry.set_user_fault(self.error_msg) + + # For failures: service side error, client side error, unexpected error + else: + telemetry.set_failure(self.error_msg) + + # For unexpected error + if self.raw_exception: + telemetry.set_exception(self.raw_exception, '') + + +def handle_exception(ex): # pylint: disable=too-many-statements # For error code, follow guidelines at https://docs.python.org/2/library/sys.html#sys.exit, from jmespath.exceptions import JMESPathTypeError from msrestazure.azure_exceptions import CloudError @@ -70,34 +148,41 @@ def handle_exception(ex): # pylint: disable=too-many-return-statements from azure.core.exceptions import AzureError with CommandLoggerContext(logger): - if isinstance(ex, JMESPathTypeError): - logger.error("\nInvalid jmespath query supplied for `--query`:\n%s", ex) - logger.error("To learn more about --query, please visit: " - "https://docs.microsoft.com/cli/azure/query-azure-cli?view=azure-cli-latest") - return 1 - if isinstance(ex, (CLIError, CloudError, AzureException, AzureError)): - logger.error(ex.args[0]) + error_msg = getattr(ex, 'message', str(ex)) + + if isinstance(ex, AzCLIError): + az_error = ex + + elif isinstance(ex, JMESPathTypeError): + error_msg = "Invalid jmespath query supplied for `--query`: {}".format(error_msg) + az_error = AzCLIError(AzCLIErrorType.ArgumentParseError, error_msg) + az_error.set_recommendation(QUERY_REFERENCE) + + elif isinstance(ex, ValidationError): + az_error = AzCLIError(AzCLIErrorType.ValidationError, error_msg) + + # TODO: Fine-grained analysis to decide whether they are ValidationErrors + elif isinstance(ex, (CLIError, CloudError, AzureError)): try: + error_msg = ex.args[0] for detail in ex.args[0].error.details: - logger.error(detail) - except (AttributeError, TypeError): - pass - except: # pylint: disable=bare-except + error_msg += ('\n' + detail) + except Exception: # pylint: disable=broad-except pass - return ex.args[1] if len(ex.args) >= 2 else 1 - if isinstance(ex, ValidationError): - logger.error('validation error: %s', ex) - return 1 - if isinstance(ex, ClientRequestError): - msg = str(ex) - if 'SSLError' in msg: - logger.error("request failed: %s", SSLERROR_TEMPLATE.format(msg)) - else: - logger.error("request failed: %s", ex) - return 1 - if isinstance(ex, KeyboardInterrupt): - return 1 - if isinstance(ex, HttpOperationError): + az_error = AzCLIError(AzCLIErrorType.ValidationError, error_msg) + + # TODO: Fine-grained analysis + elif isinstance(ex, AzureException): + az_error = AzCLIError(AzCLIErrorType.ServiceError, error_msg) + + # TODO: Fine-grained analysis + elif isinstance(ex, ClientRequestError): + az_error = AzCLIError(AzCLIErrorType.ClientError, error_msg) + if 'SSLError' in error_msg: + az_error.set_recommendation(SSLERROR_TEMPLATE) + + # TODO: Fine-grained analysis + elif isinstance(ex, HttpOperationError): try: response_dict = json.loads(ex.response.text) error = response_dict['error'] @@ -107,17 +192,27 @@ def handle_exception(ex): # pylint: disable=too-many-return-statements if isinstance(error, dict): code = "{} - ".format(error.get('code', 'Unknown Code')) message = error.get('message', ex) - logger.error("%s%s", code, message) + error_msg = "code: {}, {}".format(code, message) else: - logger.error(error) + error_msg = error except (ValueError, KeyError): - logger.error(ex) - return 1 + pass + + az_error = AzCLIError(AzCLIErrorType.ServiceError, error_msg) + + elif isinstance(ex, KeyboardInterrupt): + error_msg = 'Keyboard interrupt is captured.' + az_error = AzCLIError(AzCLIErrorType.ManualInterrupt, error_msg) + + else: + error_msg = "The command failed with an unexpected error. Here is the traceback:" + az_error = AzCLIError(AzCLIErrorType.UnexpectedError, error_msg) + az_error.set_raw_exception(ex) + az_error.set_recommendation("To open an issue, please run: 'az feedback'") - logger.error("The command failed with an unexpected error. Here is the traceback:\n") - logger.exception(ex) - logger.warning("\nTo open an issue, please run: 'az feedback'") + az_error.print_error() + az_error.send_telemetry() return 1 diff --git a/src/azure-cli/azure/cli/__main__.py b/src/azure-cli/azure/cli/__main__.py index 06eef18cc6f..0c4bad155fa 100644 --- a/src/azure-cli/azure/cli/__main__.py +++ b/src/azure-cli/azure/cli/__main__.py @@ -44,17 +44,13 @@ def cli_main(cli, args): exit_code = cli_main(az_cli, sys.argv[1:]) - if exit_code and exit_code != 0: - if az_cli.result.error is not None and not telemetry.has_exceptions(): - telemetry.set_exception(az_cli.result.error, fault_type='') - telemetry.set_failure() - else: + if exit_code == 0: telemetry.set_success() sys.exit(exit_code) except KeyboardInterrupt: - telemetry.set_user_fault('keyboard interrupt') + telemetry.set_user_fault('Keyboard interrupt is captured.') sys.exit(1) except SystemExit as ex: # some code directly call sys.exit, this is to make sure command metadata is logged exit_code = ex.code if ex.code is not None else 1 From e6d9a908909bae97be3eff29ffd04e261f3261a0 Mon Sep 17 00:00:00 2001 From: houk-ms Date: Wed, 19 Aug 2020 17:04:58 +0800 Subject: [PATCH 02/20] refine error message for SSLError --- src/azure-cli-core/azure/cli/core/util.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/util.py b/src/azure-cli-core/azure/cli/core/util.py index 01fb6f145d8..bae3f1b2764 100644 --- a/src/azure-cli-core/azure/cli/core/util.py +++ b/src/azure-cli-core/azure/cli/core/util.py @@ -27,7 +27,9 @@ COMPONENT_PREFIX = 'azure-cli-' SSLERROR_TEMPLATE = ('Certificate verification failed. This typically happens when using Azure CLI behind a proxy ' - 'that intercepts traffic with a self-signed certificate. ') + 'that intercepts traffic with a self-signed certificate. ' + # pylint: disable=line-too-long + 'Please add this certificate to the trusted CA bundle: https://github.com/Azure/azure-cli/blob/dev/doc/use_cli_effectively.md#work-behind-a-proxy. ') QUERY_REFERENCE = ("To learn more about --query, please visit: " "'https://docs.microsoft.com/cli/azure/query-azure-cli?view=azure-cli-latest'") @@ -38,7 +40,6 @@ _CHILDREN_RE = re.compile('(?i)/(?P[^/]*)/(?P[^/]*)') -# pylint: disable=line-too-long _PACKAGE_UPGRADE_INSTRUCTIONS = {"YUM": ("sudo yum update -y azure-cli", "https://aka.ms/doc/UpdateAzureCliYum"), "ZYPPER": ("sudo zypper refresh && sudo zypper update -y azure-cli", "https://aka.ms/doc/UpdateAzureCliZypper"), "DEB": ("sudo apt-get update && sudo apt-get install --only-upgrade -y azure-cli", "https://aka.ms/doc/UpdateAzureCliApt"), From 9bd6ce7cb70a8d75ac9b31269102cc8444100ee8 Mon Sep 17 00:00:00 2001 From: houk-ms Date: Thu, 20 Aug 2020 17:29:17 +0800 Subject: [PATCH 03/20] fix test issues --- .../azure/cli/core/commands/__init__.py | 13 ------------- .../latest/test_batch_data_plane_commands.py | 18 +++++++++--------- .../tests/latest/test_network_commands.py | 2 +- .../resource/tests/latest/test_resource.py | 2 +- .../latest/test_storage_account_scenarios.py | 18 +++++++----------- .../latest/test_storage_blob_scenarios.py | 6 +++--- 6 files changed, 21 insertions(+), 38 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/commands/__init__.py b/src/azure-cli-core/azure/cli/core/commands/__init__.py index 5c063013c29..ceba5919060 100644 --- a/src/azure-cli-core/azure/cli/core/commands/__init__.py +++ b/src/azure-cli-core/azure/cli/core/commands/__init__.py @@ -863,19 +863,6 @@ def _validate_arg_level(self, ns, **_): # pylint: disable=no-self-use except AttributeError: pass - def _validation(self, parsed_ns): - try: - cmd_validator = getattr(parsed_ns, '_command_validator', None) - if cmd_validator: - self._validate_cmd_level(parsed_ns, cmd_validator) - else: - self._validate_arg_level(parsed_ns) - except CLIError: - raise - except Exception: # pylint: disable=broad-except - err = sys.exc_info()[1] - getattr(parsed_ns, '_parser', self.parser).validation_error(str(err)) - class LongRunningOperation: # pylint: disable=too-few-public-methods def __init__(self, cli_ctx, start_msg='', finish_msg='', poller_done_interval_ms=1000.0): diff --git a/src/azure-cli/azure/cli/command_modules/batch/tests/latest/test_batch_data_plane_commands.py b/src/azure-cli/azure/cli/command_modules/batch/tests/latest/test_batch_data_plane_commands.py index fedfa0d9c3d..cd75f6c868b 100644 --- a/src/azure-cli/azure/cli/command_modules/batch/tests/latest/test_batch_data_plane_commands.py +++ b/src/azure-cli/azure/cli/command_modules/batch/tests/latest/test_batch_data_plane_commands.py @@ -219,7 +219,7 @@ def test_batch_jobs_and_tasks(self, resource_group, batch_account_name): # test create job with missing parameters self.kwargs['start'] = datetime.datetime.now().isoformat() - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.batch_cmd('batch job create --id {j_id} --metadata test=value ' '--job-manager-task-environment-settings a=b ' '--job-max-task-retry-count 5 ') @@ -243,7 +243,7 @@ def test_batch_jobs_and_tasks(self, resource_group, batch_account_name): self.check('metadata[0].value', 'value')]) # test bad enum value - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.batch_cmd('batch job set --job-id {j_id} --on-all-tasks-complete badValue ') # test patch job @@ -295,26 +295,26 @@ def test_batch_pools_and_nodes( '--node-agent-sku-id "batch.node.ubuntu 16.04"') # test create pool with missing parameters - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.batch_cmd('batch pool create --id missing-params-test --os-family 4') # test create pool with missing required mutually exclusive parameters - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.batch_cmd('batch pool create --id missing-required-group-test --vm-size small') # test create pool with parameters from mutually exclusive groups - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.batch_cmd('batch pool create --id mutually-exclusive-test --vm-size small ' '--os-family 4 --image Canonical:UbuntuServer:16-LTS:latest') # test create pool with invalid vm size for IaaS - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.batch_cmd('batch pool create --id invalid-size-test --vm-size small ' '--image Canonical:UbuntuServer:16.04.0-LTS --node-agent-sku-id ' '"batch.node.ubuntu 16.04"') # test create pool with missing optional parameters - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.batch_cmd('batch pool create --id missing-optional-test --vm-size small ' '--os-family 4 --start-task-wait-for-success') @@ -326,7 +326,7 @@ def test_batch_pools_and_nodes( self.check('startTask.userIdentity.userName', 'cliTestUser')]) # test create pool from non-existant JSON file - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.batch_cmd('batch pool create --json-file batch-pool-create-missing.json') # test create pool from invalid JSON file @@ -335,7 +335,7 @@ def test_batch_pools_and_nodes( self.batch_cmd('batch pool create --json-file "{json}"') # test create pool from JSON file with additional parameters - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.kwargs['json'] = self._get_test_data_file('batch-pool-create.json').replace('\\', '\\\\') self.batch_cmd('batch pool create --json-file "{json}" --vm-size small') diff --git a/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_network_commands.py b/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_network_commands.py index 1574b28103e..6f9909947ef 100644 --- a/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_network_commands.py +++ b/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_network_commands.py @@ -396,7 +396,7 @@ def test_network_public_ip_prefix(self, resource_group): ]) # Check with unsupported IP address version: IPv5 - with self.assertRaisesRegexp(SystemExit, '2'): + with self.assertRaisesRegexp(Exception, '2'): self.cmd('network public-ip prefix create -g {rg} -n {prefix_name_ipv6} --length 127 --version IPv5') diff --git a/src/azure-cli/azure/cli/command_modules/resource/tests/latest/test_resource.py b/src/azure-cli/azure/cli/command_modules/resource/tests/latest/test_resource.py index ceb43cbeceb..2ee59c308b2 100644 --- a/src/azure-cli/azure/cli/command_modules/resource/tests/latest/test_resource.py +++ b/src/azure-cli/azure/cli/command_modules/resource/tests/latest/test_resource.py @@ -2574,7 +2574,7 @@ def test_resource_group_local_context(self): self.check('name', self.kwargs['group1']), self.check('location', self.kwargs['location']) ]) - with self.assertRaisesRegexp(SystemExit, '2'): + with self.assertRaisesRegexp(Exception, '2'): self.cmd('group delete') self.cmd('group delete -n {group1} -y') self.cmd('group create -n {group2}', checks=[ diff --git a/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_account_scenarios.py b/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_account_scenarios.py index e5569e518e8..0e0d2a60153 100644 --- a/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_account_scenarios.py +++ b/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_account_scenarios.py @@ -360,7 +360,7 @@ def test_show_usage(self): self.cmd('storage account show-usage -l westus', checks=JMESPathCheck('name.value', 'StorageAccounts')) def test_show_usage_no_location(self): - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.cmd('storage account show-usage') @ResourceGroupPreparer() @@ -586,10 +586,6 @@ def test_storage_account_show_exit_codes(self, resource_group, storage_account): self.assertEqual(self.cmd('storage account show -g {rg} -n {sa}').exit_code, 0) - with self.assertRaises(SystemExit) as ex: - self.cmd('storage account show text_causing_parsing_error') - self.assertEqual(ex.exception.code, 2) - with self.assertRaises(SystemExit) as ex: self.cmd('storage account show -g fake_group -n {sa}') self.assertEqual(ex.exception.code, 3) @@ -914,22 +910,22 @@ def test_storage_account_update_delete_retention_policy(self, resource_group, st 'cmd': 'storage account blob-service-properties update' }) - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.cmd('{cmd} --enable-delete-retention true -n {sa} -g {rg}') - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.cmd('{cmd} --enable-delete-retention false --delete-retention-days 365 -n {sa} -g {rg}').get_output_in_json() - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.cmd('{cmd} --delete-retention-days 1 -n {sa} -g {rg}').get_output_in_json() - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.cmd('{cmd} --enable-delete-retention true --delete-retention-days -1 -n {sa} -g {rg}') - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.cmd('{cmd} --enable-delete-retention true --delete-retention-days 0 -n {sa} -g {rg}') - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.cmd('{cmd} --enable-delete-retention true --delete-retention-days 366 -n {sa} -g {rg}') result = self.cmd('{cmd} --enable-delete-retention true --delete-retention-days 1 -n {sa} -g {rg}').get_output_in_json() diff --git a/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_scenarios.py b/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_scenarios.py index 57b6d2c3003..d82736f47cf 100644 --- a/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_scenarios.py +++ b/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_scenarios.py @@ -522,7 +522,7 @@ def test_storage_page_blob_set_tier(self, resource_group, storage_account): self.storage_cmd('az storage blob show -c {} -n {} ', account_info, container_name, blob_name)\ .assert_with_checks(JMESPathCheck('properties.blobTier', 'P10')) - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.storage_cmd('storage blob set-tier -c {} -n {} --tier P20 -r High -t page', account_info, container_name, blob_name) @@ -546,11 +546,11 @@ def test_storage_block_blob_set_tier(self, resource_group, storage_account): self.storage_cmd('storage blob upload -c {} -n {} -f "{}"', account_info, container_name, blob_name, source_file) - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.storage_cmd('storage blob set-tier -c {} -n {} --tier Cool -r Middle', account_info, container_name, blob_name) - with self.assertRaises(SystemExit): + with self.assertRaises(Exception): self.storage_cmd('storage blob set-tier -c {} -n {} --tier Archive -r High', account_info, container_name, blob_name) From 2a27fab76758cf9b58310a5c4a4b03f290520a88 Mon Sep 17 00:00:00 2001 From: houk-ms Date: Thu, 20 Aug 2020 18:13:26 +0800 Subject: [PATCH 04/20] fix test issue for 2019-03-01-hybrid --- .../tests/hybrid_2019_03_01/test_storage_account_scenarios.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/storage/tests/hybrid_2019_03_01/test_storage_account_scenarios.py b/src/azure-cli/azure/cli/command_modules/storage/tests/hybrid_2019_03_01/test_storage_account_scenarios.py index d9212390261..8a5cb667d31 100644 --- a/src/azure-cli/azure/cli/command_modules/storage/tests/hybrid_2019_03_01/test_storage_account_scenarios.py +++ b/src/azure-cli/azure/cli/command_modules/storage/tests/hybrid_2019_03_01/test_storage_account_scenarios.py @@ -258,10 +258,6 @@ def test_storage_account_show_exit_codes(self, resource_group, storage_account): self.assertEqual(self.cmd('storage account show -g {rg} -n {sa}').exit_code, 0) - with self.assertRaises(SystemExit) as ex: - self.cmd('storage account show text_causing_parsing_error') - self.assertEqual(ex.exception.code, 2) - with self.assertRaises(SystemExit) as ex: self.cmd('storage account show -g fake_group -n {sa}') self.assertEqual(ex.exception.code, 3) From 2d906ffaddf136a9773eeeb5be23aa269ad2cec1 Mon Sep 17 00:00:00 2001 From: houk-ms Date: Wed, 26 Aug 2020 13:09:22 +0800 Subject: [PATCH 05/20] split AzCLIError and CommandRecommender into seperate files --- src/azure-cli-core/azure/cli/core/_help.py | 4 + .../azure/cli/core/azclierror.py | 88 ++++++++ .../azure/cli/core/command_recommender.py | 190 ++++++++++++++++++ .../azure/cli/core/commands/__init__.py | 4 +- .../azure/cli/core/commands/arm.py | 4 +- src/azure-cli-core/azure/cli/core/parser.py | 184 +---------------- src/azure-cli-core/azure/cli/core/util.py | 84 +------- 7 files changed, 295 insertions(+), 263 deletions(-) create mode 100644 src/azure-cli-core/azure/cli/core/azclierror.py create mode 100644 src/azure-cli-core/azure/cli/core/command_recommender.py diff --git a/src/azure-cli-core/azure/cli/core/_help.py b/src/azure-cli-core/azure/cli/core/_help.py index 46d9c4e7888..d1e568bfb8d 100644 --- a/src/azure-cli-core/azure/cli/core/_help.py +++ b/src/azure-cli-core/azure/cli/core/_help.py @@ -184,6 +184,10 @@ def show_help(self, cli_name, nouns, parser, is_group): print(UX_SURVEY_PROMPT_COLOR if self.cli_ctx.enable_color else UX_SURVEY_PROMPT) def get_examples(self, command, parser, is_group): + """Get examples of a certain command from the help file. + Get the text of the example, strip the newline character and + return a list of commands which start with the given command name. + """ nouns = command.split(' ')[1:] self.update_loaders_with_help_file_contents(nouns) diff --git a/src/azure-cli-core/azure/cli/core/azclierror.py b/src/azure-cli-core/azure/cli/core/azclierror.py new file mode 100644 index 00000000000..f1250155655 --- /dev/null +++ b/src/azure-cli-core/azure/cli/core/azclierror.py @@ -0,0 +1,88 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +import sys +from enum import Enum + +from knack.util import CLIError +from knack.log import get_logger + +logger = get_logger(__name__) + + +class AzCLIErrorType(Enum): + """ AzureCLI error types """ + + # userfaults + CommandNotFoundError = 'CommandNotFoundError' + ArgumentParseError = 'ArgumentParseError' + ValidationError = 'ValidationError' + ManualInterrupt = 'ManualInterrupt' + # service side error + ServiceError = 'ServiceError' + # client side error + ClientError = 'ClientError' + # unexpected error + UnexpectedError = 'UnexpectedError' + + +class AzCLIError(CLIError): + """ AzureCLI error definition """ + + def __init__(self, error_type, error_msg, raw_exception=None, command=None): + """ + :param error_type: The name of the AzureCLI error type. + :type error_type: azure.cli.core.util.AzCLIErrorType + :param error_msg: The error message detail. + :type error_msg: str + :param raw_exception: The raw exception. + :type raw_exception: Exception + :param command: The command which brings the error. + :type command: str + :param recommendations: The recommendations to resolve the error. + :type recommendations: list + """ + self.error_type = error_type + self.error_msg = error_msg + self.raw_exception = raw_exception + self.command = command + self.recommendations = [] + super().__init__(error_msg) + + def set_recommendation(self, recommendation): + self.recommendations.append(recommendation) + + def set_raw_exception(self, raw_exception): + self.raw_exception = raw_exception + + def print_error(self): + from azure.cli.core.azlogging import CommandLoggerContext + with CommandLoggerContext(logger): + message = '{}: {}'.format(self.error_type.value, self.error_msg) + logger.error(message) + if self.raw_exception: + logger.exception(self.raw_exception) + if self.recommendations: + for recommendation in self.recommendations: + print(recommendation, file=sys.stderr) + + def send_telemetry(self): + import azure.cli.core.telemetry as telemetry + telemetry.set_error_type(self.error_type.value) + + # For userfaults + if self.error_type in [AzCLIErrorType.CommandNotFoundError, + AzCLIErrorType.ArgumentParseError, + AzCLIErrorType.ValidationError, + AzCLIErrorType.ManualInterrupt]: + telemetry.set_user_fault(self.error_msg) + + # For failures: service side error, client side error, unexpected error + else: + telemetry.set_failure(self.error_msg) + + # For unexpected error + if self.raw_exception: + telemetry.set_exception(self.raw_exception, '') diff --git a/src/azure-cli-core/azure/cli/core/command_recommender.py b/src/azure-cli-core/azure/cli/core/command_recommender.py new file mode 100644 index 00000000000..e854f13d556 --- /dev/null +++ b/src/azure-cli-core/azure/cli/core/command_recommender.py @@ -0,0 +1,190 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +import difflib + +import azure.cli.core.telemetry as telemetry +from knack.log import get_logger + +logger = get_logger(__name__) + + +class CommandRecommender(): + """Recommend a command for user when user's command fails. + It combines alladin recommendations and examples in help files.""" + + def __init__(self, command, parameters, extension, cli_ctx): + """ + :param command: The command name in user's input, which should be corrected if misspelled. + :type command: str + :param parameters: The parameter arguments in users input. + :type parameters: list + :param extension: The extension name in user's input if the command comes from an extension. + :type extension: str + :param cli_ctx: CLI context when parser fails. + :type cli_ctx: knack.cli.CLI + """ + self.command = command.strip() + self.extension = extension + self.cli_ctx = cli_ctx + + self.parameters = self._normalize_parameters(parameters) + self.help_examples = [] + self.aladdin_recommendations = [] + + def set_help_examples(self, examples): + """Set recommendations from help files""" + + self.help_examples.extend(examples) + + def _set_aladdin_recommendations(self): + """Set recommendations from aladdin service. + Call the aladdin service API, parse the response and set the recommendations. + """ + + import json + import requests + from requests import RequestException + from http import HTTPStatus + from azure.cli.core import __version__ as version + + api_url = 'https://app.aladdin.microsoft.com/api/v1.0/suggestions' + headers = {'Content-Type': 'application/json'} + correlation_id = telemetry._session.correlation_id # pylint: disable=protected-access + subscription_id = telemetry._get_azure_subscription_id() # pylint: disable=protected-access + + context = { + 'versionNumber': version + } + if telemetry.is_telemetry_enabled(): + if correlation_id: + context['correlationId'] = correlation_id + if subscription_id: + context['subscriptionId'] = subscription_id + + parameters = [item for item in self.parameters if item not in ['--debug', '--verbose']] + query = { + "command": self.command, + "parameters": ','.join(parameters) + } + + response = None + try: + response = requests.get( + api_url, + params={ + 'query': json.dumps(query), + 'clientType': 'AzureCli', + 'context': json.dumps(context) + }, + headers=headers) + except RequestException as ex: + logger.debug('Recommendation requests.get() exception: %s', ex) + + recommendations = [] + if response and response.status_code == HTTPStatus.OK: + for result in response.json(): + # parse the reponse and format the recommendation + command, parameters, placeholders = result['SuccessCommand'],\ + result['SuccessCommand_Parameters'].split(','),\ + result['SuccessCommand_ArgumentPlaceholders'].split('♠') + recommendation = 'az {} '.format(command) + for parameter, placeholder in zip(parameters, placeholders): + recommendation += '{} {} '.format(parameter, placeholder) + recommendations.append(recommendation.strip()) + + self.aladdin_recommendations.extend(recommendations) + + def recommend_a_command(self): + """Recommend a command for user when user's command fails. + The recommended command will be the best matched one from + both the help files and the aladdin recommendations. + """ + self._set_aladdin_recommendations() + # all the recommended commands from help examples and aladdin + all_commands = self.help_examples + self.aladdin_recommendations + all_commands.sort(key=len) + + filtered_commands = [] + filtered_choices = [] + target = ''.join(self.parameters) + + for command in all_commands: + # filter out the commands which begins with a different command group + if command.startswith('az {}'.format(self.command)): + parameters = self._get_parameter_list(command) + normalized_parameters = self._normalize_parameters(parameters) + filtered_choices.append(''.join(normalized_parameters)) + filtered_commands.append(command) + + # sort the commands by argument matches + candidates = difflib.get_close_matches(target, filtered_choices, cutoff=0) + + recommend_command = '' + if candidates: + index = filtered_choices.index(candidates[0]) + recommend_command = filtered_commands[index] + + return recommend_command + + def _get_parameter_list(self, raw_command): # pylint: disable=no-self-use + """Get the paramter list from a raw command string + An example: 'az group create -n test -l eastus' ==> ['-n', '-l'] + """ + contents = raw_command.split(' ') + return [item for item in contents if item.startswith('-')] + + def _normalize_parameters(self, parameters): + """Normalize a parameter list. + Get the standard form of a parameter list, which includes: + 1. Use long options to replace short options + 2. Remove the unrecognized parameters + 3. Sort the result parameter list + An example: ['-g', '-n'] ==> ['--name', '--resource-group'] + """ + + from knack.deprecation import Deprecated + + normalized_parameters = [] + try: + cmd_table = self.cli_ctx.invocation.commands_loader.command_table.get(self.command, None) + parameter_table = cmd_table.arguments if cmd_table else None + except AttributeError: + parameter_table = None + + if parameters: + rules = { + '-h': '--help', + '-o': '--output', + '--only-show-errors': None, + '--help': None, + '--output': None, + '--query': None, + '--debug': None, + '--verbose': None + } + + if parameter_table: + for argument in parameter_table.values(): + options = argument.type.settings['options_list'] + options = (option for option in options if not isinstance(option, Deprecated)) + try: + sorted_options = sorted(options, key=len, reverse=True) + standard_form = sorted_options[0] + + for option in sorted_options[1:]: + rules[option] = standard_form + rules[standard_form] = None + except TypeError: + logger.debug('Unexpected argument options `%s` of type `%s`.', options, type(options).__name__) + + for parameter in parameters: + if parameter in rules: + normalized_form = rules.get(parameter, None) or parameter + normalized_parameters.append(normalized_form) + else: + logger.debug('"%s" is an invalid parameter for command "%s".', parameter, self.command) + + return sorted(normalized_parameters) diff --git a/src/azure-cli-core/azure/cli/core/commands/__init__.py b/src/azure-cli-core/azure/cli/core/commands/__init__.py index ceba5919060..dfdae21111e 100644 --- a/src/azure-cli-core/azure/cli/core/commands/__init__.py +++ b/src/azure-cli-core/azure/cli/core/commands/__init__.py @@ -845,8 +845,8 @@ def _validate_cmd_level(self, ns, cmd_validator): # pylint: disable=no-self-use pass def _validate_arg_level(self, ns, **_): # pylint: disable=no-self-use - from azure.cli.core.util import AzCLIErrorType - from azure.cli.core.util import AzCLIError + from azure.cli.core.azclierror import AzCLIErrorType + from azure.cli.core.azclierror import AzCLIError for validator in getattr(ns, '_argument_validators', []): try: validator(**self._build_kwargs(validator, ns)) diff --git a/src/azure-cli-core/azure/cli/core/commands/arm.py b/src/azure-cli-core/azure/cli/core/commands/arm.py index 3f6f15e1370..46cd0334d37 100644 --- a/src/azure-cli-core/azure/cli/core/commands/arm.py +++ b/src/azure-cli-core/azure/cli/core/commands/arm.py @@ -758,8 +758,8 @@ def show_exception_handler(ex): if getattr(getattr(ex, 'response', ex), 'status_code', None) == 404: import sys from azure.cli.core.azlogging import CommandLoggerContext - from azure.cli.core.util import AzCLIErrorType - from azure.cli.core.util import AzCLIError + from azure.cli.core.azclierror import AzCLIErrorType + from azure.cli.core.azclierror import AzCLIError with CommandLoggerContext(logger): az_error = AzCLIError(AzCLIErrorType.ValidationError, getattr(ex, 'message', ex)) az_error.print_error() diff --git a/src/azure-cli-core/azure/cli/core/parser.py b/src/azure-cli-core/azure/cli/core/parser.py index aac956da229..82c105f0382 100644 --- a/src/azure-cli-core/azure/cli/core/parser.py +++ b/src/azure-cli-core/azure/cli/core/parser.py @@ -17,8 +17,9 @@ from azure.cli.core.commands import ExtensionCommandSource from azure.cli.core.commands import AzCliCommandInvoker from azure.cli.core.commands.events import EVENT_INVOKER_ON_TAB_COMPLETION -from azure.cli.core.util import AzCLIErrorType -from azure.cli.core.util import AzCLIError +from azure.cli.core.azclierror import AzCLIErrorType +from azure.cli.core.azclierror import AzCLIError +from azure.cli.core.command_recommender import CommandRecommender from knack.log import get_logger from knack.parser import CLICommandParser @@ -61,185 +62,6 @@ def _get_completions(self, comp_words, cword_prefix, cword_prequote, last_wordbr last_wordbreak_pos) -class CommandRecommender(): - """Recommend a command for user when user's command fails. - It combines alladin recommendations and examples in help files.""" - - def __init__(self, command, parameters, extension, cli_ctx): - """ - :param command: The command name in user's input, which should be corrected if misspelled. - :type command: str - :param parameters: The parameter arguments in users input. - :type parameters: list - :param extension: The extension name in user's input if the command comes from an extension. - :type extension: str - :param cli_ctx: CLI context when parser fails. - :type cli_ctx: knack.cli.CLI - """ - self.command = command.strip() - self.extension = extension - self.cli_ctx = cli_ctx - - self.parameters = self._normalize_parameters(parameters) - self.help_examples = [] - self.aladdin_recommendations = [] - - def set_help_examples(self, examples): - """Set recommendations from help files""" - - self.help_examples.extend(examples) - - def _set_aladdin_recommendations(self): - """Set recommendations from aladdin service. - Call the aladdin service API, parse the response and set the recommendations. - """ - - import json - import requests - from requests import RequestException - from http import HTTPStatus - from azure.cli.core import __version__ as version - - api_url = 'https://app.aladdin.microsoft.com/api/v1.0/suggestions' - headers = {'Content-Type': 'application/json'} - correlation_id = telemetry._session.correlation_id # pylint: disable=protected-access - subscription_id = telemetry._get_azure_subscription_id() # pylint: disable=protected-access - - context = { - 'versionNumber': version - } - if telemetry.is_telemetry_enabled(): - if correlation_id: - context['correlationId'] = correlation_id - if subscription_id: - context['subscriptionId'] = subscription_id - - parameters = [item for item in self.parameters if item not in ['--debug', '--verbose']] - query = { - "command": self.command, - "parameters": ','.join(parameters) - } - - response = None - try: - response = requests.get( - api_url, - params={ - 'query': json.dumps(query), - 'clientType': 'AzureCli', - 'context': json.dumps(context) - }, - headers=headers) - except RequestException as ex: - logger.debug('Recommendation requests.get() exception: %s', ex) - - recommendations = [] - if response and response.status_code == HTTPStatus.OK: - for result in response.json(): - # parse the reponse and format the recommendation - command, parameters, placeholders = result['SuccessCommand'],\ - result['SuccessCommand_Parameters'].split(','),\ - result['SuccessCommand_ArgumentPlaceholders'].split('♠') - recommendation = 'az {} '.format(command) - for parameter, placeholder in zip(parameters, placeholders): - recommendation += '{} {} '.format(parameter, placeholder) - recommendations.append(recommendation.strip()) - - self.aladdin_recommendations.extend(recommendations) - - def recommend_a_command(self): - """Recommend a command for user when user's command fails. - The recommended command will be the best matched one from - both the help files and the aladdin recommendations. - """ - self._set_aladdin_recommendations() - # all the recommended commands from help examples and aladdin - all_commands = self.help_examples + self.aladdin_recommendations - all_commands.sort(key=len) - - filtered_commands = [] - filtered_choices = [] - target = ''.join(self.parameters) - - for command in all_commands: - # filter out the commands which begins with a different command group - if command.startswith('az {}'.format(self.command)): - parameters = self._get_parameter_list(command) - normalized_parameters = self._normalize_parameters(parameters) - filtered_choices.append(''.join(normalized_parameters)) - filtered_commands.append(command) - - # sort the commands by argument matches - candidates = difflib.get_close_matches(target, filtered_choices, cutoff=0) - - recommend_command = '' - if candidates: - index = filtered_choices.index(candidates[0]) - recommend_command = filtered_commands[index] - - return recommend_command - - def _get_parameter_list(self, raw_command): # pylint: disable=no-self-use - """Get the paramter list from a raw command string - An example: 'az group create -n test -l eastus' ==> ['-n', '-l'] - """ - contents = raw_command.split(' ') - return [item for item in contents if item.startswith('-')] - - def _normalize_parameters(self, parameters): - """Normalize a parameter list. - Get the standard form of a parameter list, which includes: - 1. Use long options to replace short options - 2. Remove the unrecognized parameters - 3. Sort the result parameter list - An example: ['-g', '-n'] ==> ['--name', '--resource-group'] - """ - - from knack.deprecation import Deprecated - - normalized_parameters = [] - try: - cmd_table = self.cli_ctx.invocation.commands_loader.command_table.get(self.command, None) - parameter_table = cmd_table.arguments if cmd_table else None - except AttributeError: - parameter_table = None - - if parameters: - rules = { - '-h': '--help', - '-o': '--output', - '--only-show-errors': None, - '--help': None, - '--output': None, - '--query': None, - '--debug': None, - '--verbose': None - } - - if parameter_table: - for argument in parameter_table.values(): - options = argument.type.settings['options_list'] - options = (option for option in options if not isinstance(option, Deprecated)) - try: - sorted_options = sorted(options, key=len, reverse=True) - standard_form = sorted_options[0] - - for option in sorted_options[1:]: - rules[option] = standard_form - rules[standard_form] = None - except TypeError: - logger.debug('Unexpected argument options `%s` of type `%s`.', options, type(options).__name__) - - for parameter in parameters: - if parameter in rules: - normalized_form = rules.get(parameter, None) or parameter - normalized_parameters.append(normalized_form) - else: - logger.debug('"%s" is an invalid parameter for command "%s".', parameter, self.command) - - return sorted(normalized_parameters) - - class AzCliCommandParser(CLICommandParser): """ArgumentParser implementation specialized for the Azure CLI utility.""" diff --git a/src/azure-cli-core/azure/cli/core/util.py b/src/azure-cli-core/azure/cli/core/util.py index bae3f1b2764..ad727baa4fc 100644 --- a/src/azure-cli-core/azure/cli/core/util.py +++ b/src/azure-cli-core/azure/cli/core/util.py @@ -14,10 +14,11 @@ import ssl import re import logging -from enum import Enum import six from six.moves.urllib.request import urlopen # pylint: disable=import-error +from azure.cli.core.azclierror import AzCLIErrorType +from azure.cli.core.azclierror import AzCLIError from knack.log import get_logger from knack.util import CLIError, to_snake_case @@ -63,82 +64,6 @@ ] -class AzCLIErrorType(Enum): - """ AzureCLI error types """ - - # userfaults - CommandNotFoundError = 'CommandNotFoundError' - ArgumentParseError = 'ArgumentParseError' - ValidationError = 'ValidationError' - ManualInterrupt = 'ManualInterrupt' - # service side error - ServiceError = 'ServiceError' - # client side error - ClientError = 'ClientError' - # unexpected error - UnexpectedError = 'UnexpectedError' - - -class AzCLIError(CLIError): - """ AzureCLI error definition """ - - def __init__(self, error_type, error_msg, raw_exception=None, command=None): - """ - :param error_type: The name of the AzureCLI error type. - :type error_type: azure.cli.core.util.AzCLIErrorType - :param error_msg: The error message detail. - :type error_msg: str - :param raw_exception: The raw exception. - :type raw_exception: Exception - :param command: The command which brings the error. - :type command: str - :param recommendations: The recommendations to resolve the error. - :type recommendations: list - """ - self.error_type = error_type - self.error_msg = error_msg - self.raw_exception = raw_exception - self.command = command - self.recommendations = [] - super().__init__(error_msg) - - def set_recommendation(self, recommendation): - self.recommendations.append(recommendation) - - def set_raw_exception(self, raw_exception): - self.raw_exception = raw_exception - - def print_error(self): - from azure.cli.core.azlogging import CommandLoggerContext - with CommandLoggerContext(logger): - message = '{}: {}'.format(self.error_type.value, self.error_msg) - logger.error(message) - if self.raw_exception: - logger.exception(self.raw_exception) - if self.recommendations: - for recommendation in self.recommendations: - print(recommendation, file=sys.stderr) - - def send_telemetry(self): - import azure.cli.core.telemetry as telemetry - telemetry.set_error_type(self.error_type.value) - - # For userfaults - if self.error_type in [AzCLIErrorType.CommandNotFoundError, - AzCLIErrorType.ArgumentParseError, - AzCLIErrorType.ValidationError, - AzCLIErrorType.ManualInterrupt]: - telemetry.set_user_fault(self.error_msg) - - # For failures: service side error, client side error, unexpected error - else: - telemetry.set_failure(self.error_msg) - - # For unexpected error - if self.raw_exception: - telemetry.set_exception(self.raw_exception, '') - - def handle_exception(ex): # pylint: disable=too-many-statements # For error code, follow guidelines at https://docs.python.org/2/library/sys.html#sys.exit, from jmespath.exceptions import JMESPathTypeError @@ -150,6 +75,7 @@ def handle_exception(ex): # pylint: disable=too-many-statements with CommandLoggerContext(logger): error_msg = getattr(ex, 'message', str(ex)) + exit_code = 1 if isinstance(ex, AzCLIError): az_error = ex @@ -171,10 +97,12 @@ def handle_exception(ex): # pylint: disable=too-many-statements except Exception: # pylint: disable=broad-except pass az_error = AzCLIError(AzCLIErrorType.ValidationError, error_msg) + exit_code = ex.args[1] if len(ex.args) >= 2 else 1 # TODO: Fine-grained analysis elif isinstance(ex, AzureException): az_error = AzCLIError(AzCLIErrorType.ServiceError, error_msg) + exit_code = ex.args[1] if len(ex.args) >= 2 else 1 # TODO: Fine-grained analysis elif isinstance(ex, ClientRequestError): @@ -215,7 +143,7 @@ def handle_exception(ex): # pylint: disable=too-many-statements az_error.print_error() az_error.send_telemetry() - return 1 + return exit_code # pylint: disable=inconsistent-return-statements From 1fea90f65e74b6085c7bd755db06275e78724a34 Mon Sep 17 00:00:00 2001 From: houk-ms Date: Wed, 26 Aug 2020 13:53:04 +0800 Subject: [PATCH 06/20] add timeout for CommandRecommender --- .../azure/cli/core/command_recommender.py | 3 ++- .../azure/cli/core/tests/test_util.py | 16 ++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/command_recommender.py b/src/azure-cli-core/azure/cli/core/command_recommender.py index e854f13d556..9bf66ab416f 100644 --- a/src/azure-cli-core/azure/cli/core/command_recommender.py +++ b/src/azure-cli-core/azure/cli/core/command_recommender.py @@ -79,7 +79,8 @@ def _set_aladdin_recommendations(self): 'clientType': 'AzureCli', 'context': json.dumps(context) }, - headers=headers) + headers=headers, + timeout=1) except RequestException as ex: logger.debug('Recommendation requests.get() exception: %s', ex) diff --git a/src/azure-cli-core/azure/cli/core/tests/test_util.py b/src/azure-cli-core/azure/cli/core/tests/test_util.py index 8d5db93546a..0ea8a16d9d4 100644 --- a/src/azure-cli-core/azure/cli/core/tests/test_util.py +++ b/src/azure-cli-core/azure/cli/core/tests/test_util.py @@ -392,7 +392,7 @@ def test_b64_to_hex_type(self): class TestHandleException(unittest.TestCase): - @mock.patch('azure.cli.core.util.logger.error', autospec=True) + @mock.patch('azure.cli.core.azclierror.logger.error', autospec=True) def test_handle_exception_keyboardinterrupt(self, mock_logger_error): # create test KeyboardInterrupt Exception keyboard_interrupt_ex = KeyboardInterrupt("KeyboardInterrupt") @@ -404,7 +404,7 @@ def test_handle_exception_keyboardinterrupt(self, mock_logger_error): self.assertTrue(mock_logger_error.called) self.assertEqual(ex_result, 1) - @mock.patch('azure.cli.core.util.logger.error', autospec=True) + @mock.patch('azure.cli.core.azclierror.logger.error', autospec=True) def test_handle_exception_clierror(self, mock_logger_error): from knack.util import CLIError @@ -420,7 +420,7 @@ def test_handle_exception_clierror(self, mock_logger_error): self.assertIn(err_msg, mock_logger_error.call_args.args[0]) self.assertEqual(ex_result, 1) - @mock.patch('azure.cli.core.util.logger.error', autospec=True) + @mock.patch('azure.cli.core.azclierror.logger.error', autospec=True) def test_handle_exception_clouderror(self, mock_logger_error): from msrestazure.azure_exceptions import CloudError @@ -436,9 +436,9 @@ def test_handle_exception_clouderror(self, mock_logger_error): # test behavior self.assertTrue(mock_logger_error.called) self.assertIn(mock_cloud_error.args[0], mock_logger_error.call_args.args[0]) - self.assertEqual(ex_result, 1) + self.assertEqual(ex_result, mock_cloud_error.args[1]) - @mock.patch('azure.cli.core.util.logger.error', autospec=True) + @mock.patch('azure.cli.core.azclierror.logger.error', autospec=True) def test_handle_exception_httpoperationerror_typical_response_error(self, mock_logger_error): # create test HttpOperationError Exception err_msg = "Bad Request because of some incorrect param" @@ -456,7 +456,7 @@ def test_handle_exception_httpoperationerror_typical_response_error(self, mock_l self.assertIn(err_code, mock_logger_error.call_args.args[0]) self.assertEqual(ex_result, 1) - @mock.patch('azure.cli.core.util.logger.error', autospec=True) + @mock.patch('azure.cli.core.azclierror.logger.error', autospec=True) def test_handle_exception_httpoperationerror_error_key_has_string_value(self, mock_logger_error): # test error in response, but has str value. @@ -476,7 +476,7 @@ def test_handle_exception_httpoperationerror_error_key_has_string_value(self, mo self.assertIn(expected_message, mock_logger_error.call_args.args[0]) self.assertEqual(ex_result, 1) - @mock.patch('azure.cli.core.util.logger.error', autospec=True) + @mock.patch('azure.cli.core.azclierror.logger.error', autospec=True) def test_handle_exception_httpoperationerror_no_error_key(self, mock_logger_error): # test error not in response @@ -494,7 +494,7 @@ def test_handle_exception_httpoperationerror_no_error_key(self, mock_logger_erro self.assertIn(str(mock.call(mock_http_error).args[0]), mock_logger_error.call_args.args[0]) self.assertEqual(ex_result, 1) - @mock.patch('azure.cli.core.util.logger.error', autospec=True) + @mock.patch('azure.cli.core.azclierror.logger.error', autospec=True) def test_handle_exception_httpoperationerror_no_response_text(self, mock_logger_error): # test no response text From 06888502590f263b34b6d91627d98dd0aedd6259 Mon Sep 17 00:00:00 2001 From: houk-ms Date: Mon, 31 Aug 2020 18:01:45 +0800 Subject: [PATCH 07/20] stop air-gapped from calling aladdin service --- src/azure-cli-core/azure/cli/core/cloud.py | 3 +++ src/azure-cli-core/azure/cli/core/command_recommender.py | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/cloud.py b/src/azure-cli-core/azure/cli/core/cloud.py index 79b997f778a..31e5b638815 100644 --- a/src/azure-cli-core/azure/cli/core/cloud.py +++ b/src/azure-cli-core/azure/cli/core/cloud.py @@ -23,6 +23,9 @@ # Add names of clouds that don't allow telemetry data collection here such as JEDI. CLOUDS_FORBIDDING_TELEMETRY = [] +# Add names of clouds that don't allow Aladdin requests for command recommendations here +CLOUDS_FORBIDDING_ALADDIN_REQUEST = ['USSec', 'USNat'] + class CloudNotRegisteredException(Exception): def __init__(self, cloud_name): diff --git a/src/azure-cli-core/azure/cli/core/command_recommender.py b/src/azure-cli-core/azure/cli/core/command_recommender.py index 9bf66ab416f..0ae82d859f7 100644 --- a/src/azure-cli-core/azure/cli/core/command_recommender.py +++ b/src/azure-cli-core/azure/cli/core/command_recommender.py @@ -103,7 +103,11 @@ def recommend_a_command(self): The recommended command will be the best matched one from both the help files and the aladdin recommendations. """ - self._set_aladdin_recommendations() + from azure.cli.core.cloud import CLOUDS_FORBIDDING_ALADDIN_REQUEST + + if self.cli_ctx.cloud.name not in CLOUDS_FORBIDDING_ALADDIN_REQUEST: + self._set_aladdin_recommendations() + # all the recommended commands from help examples and aladdin all_commands = self.help_examples + self.aladdin_recommendations all_commands.sort(key=len) From 4d79c490dfbc512c34947fdf44a8cc5c139e2e22 Mon Sep 17 00:00:00 2001 From: houk-ms Date: Mon, 31 Aug 2020 18:13:55 +0800 Subject: [PATCH 08/20] code refining --- src/azure-cli-core/azure/cli/core/command_recommender.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/command_recommender.py b/src/azure-cli-core/azure/cli/core/command_recommender.py index 0ae82d859f7..4b903b2905b 100644 --- a/src/azure-cli-core/azure/cli/core/command_recommender.py +++ b/src/azure-cli-core/azure/cli/core/command_recommender.py @@ -105,7 +105,8 @@ def recommend_a_command(self): """ from azure.cli.core.cloud import CLOUDS_FORBIDDING_ALADDIN_REQUEST - if self.cli_ctx.cloud.name not in CLOUDS_FORBIDDING_ALADDIN_REQUEST: + if self.cli_ctx and self.cli_ctx.cloud \ + and self.cli_ctx.cloud.name not in CLOUDS_FORBIDDING_ALADDIN_REQUEST: self._set_aladdin_recommendations() # all the recommended commands from help examples and aladdin From 80601f8febaa22e19a341dd4f6944b6460bd868f Mon Sep 17 00:00:00 2001 From: houk-ms Date: Fri, 11 Sep 2020 16:52:45 +0800 Subject: [PATCH 09/20] fix live test --- .../azure/cli/core/command_recommender.py | 15 ++++++++------- src/azure-cli-core/azure/cli/core/telemetry.py | 2 ++ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/command_recommender.py b/src/azure-cli-core/azure/cli/core/command_recommender.py index 921e0d47c7d..d62ffcbd89b 100644 --- a/src/azure-cli-core/azure/cli/core/command_recommender.py +++ b/src/azure-cli-core/azure/cli/core/command_recommender.py @@ -4,14 +4,15 @@ # -------------------------------------------------------------------------------------------- import difflib +from enum import Enum import azure.cli.core.telemetry as telemetry from knack.log import get_logger -from typing import Union -from enum import Enum + logger = get_logger(__name__) + class AladdinUserFaultType(Enum): """Define the userfault types required by aladdin service to get the command recommendations""" @@ -74,7 +75,7 @@ def _set_aladdin_recommendations(self): import hashlib import json import requests - from requests import RequestException, HTTPError + from requests import RequestException from http import HTTPStatus from azure.cli.core import __version__ as version @@ -91,7 +92,7 @@ def _set_aladdin_recommendations(self): } context = { 'versionNumber': version, - 'errorType': self._get_error_type().value + 'errorType': self._get_error_type() } if telemetry.is_telemetry_enabled(): @@ -241,7 +242,7 @@ def _get_error_type(self): error_type = AladdinUserFaultType.Unknown if not self.error_msg: - return error_type + return error_type.value error_msg = self.error_msg.lower() if 'unrecognized' in error_msg: @@ -265,7 +266,7 @@ def _get_error_type(self): elif 'resource_group' in error_msg or 'resource group' in error_msg: error_type = AladdinUserFaultType.ResourceGroupNotFound elif 'pattern' in error_msg or 'is not a valid value' in error_msg or 'invalid' in error_msg: - error_type = 'InvalidParameterValue' + error_type = AladdinUserFaultType.InvalidParameterValue if 'jmespath_type' in error_msg: error_type = AladdinUserFaultType.InvalidJMESPathQuery elif 'datetime_type' in error_msg: @@ -279,4 +280,4 @@ def _get_error_type(self): elif "validation error" in error_msg: error_type = AladdinUserFaultType.ValidationError - return error_type + return error_type.value diff --git a/src/azure-cli-core/azure/cli/core/telemetry.py b/src/azure-cli-core/azure/cli/core/telemetry.py index 4df97109ff8..e54ca3682d2 100644 --- a/src/azure-cli-core/azure/cli/core/telemetry.py +++ b/src/azure-cli-core/azure/cli/core/telemetry.py @@ -340,11 +340,13 @@ def set_user_fault(summary=None): if summary: _session.result_summary = _remove_cmd_chars(summary) + @decorators.suppress_all_exceptions() def set_debug_info(key, info): debug_info = '{}: {}'.format(key, info) _session.debug_info.append(debug_info) + @decorators.suppress_all_exceptions() def set_application(application, arg_complete_env_name): _session.application, _session.arg_complete_env_name = application, arg_complete_env_name From ea8c67be2ac5238cab6ca276d2cb473db9cb1f69 Mon Sep 17 00:00:00 2001 From: houk-ms Date: Mon, 14 Sep 2020 09:30:13 +0800 Subject: [PATCH 10/20] adjust aladdin response interface --- src/azure-cli-core/azure/cli/core/command_recommender.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/command_recommender.py b/src/azure-cli-core/azure/cli/core/command_recommender.py index d62ffcbd89b..f3ebaf75ea2 100644 --- a/src/azure-cli-core/azure/cli/core/command_recommender.py +++ b/src/azure-cli-core/azure/cli/core/command_recommender.py @@ -126,9 +126,9 @@ def _set_aladdin_recommendations(self): if response and response.status_code == HTTPStatus.OK: for result in response.json(): # parse the reponse and format the recommendation - command, parameters, placeholders = result['SuccessCommand'],\ - result['SuccessCommand_Parameters'].split(','),\ - result['SuccessCommand_ArgumentPlaceholders'].split('♠') + command, parameters, placeholders = result['command'],\ + result['parameters'].split(','),\ + result['placeholders'].split('♠') recommendation = 'az {} '.format(command) for parameter, placeholder in zip(parameters, placeholders): recommendation += '{} {} '.format(parameter, placeholder) From 6a97d32b47eec33497f9bebd4725e61a9609d2b3 Mon Sep 17 00:00:00 2001 From: Feng Zhou <55177366+fengzhou-msft@users.noreply.github.com> Date: Mon, 14 Sep 2020 13:19:33 +0800 Subject: [PATCH 11/20] Update cloud.py --- src/azure-cli-core/azure/cli/core/cloud.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/cloud.py b/src/azure-cli-core/azure/cli/core/cloud.py index 31e5b638815..af183e9c166 100644 --- a/src/azure-cli-core/azure/cli/core/cloud.py +++ b/src/azure-cli-core/azure/cli/core/cloud.py @@ -20,8 +20,8 @@ CLOUD_CONFIG_FILE = os.path.join(GLOBAL_CONFIG_DIR, 'clouds.config') -# Add names of clouds that don't allow telemetry data collection here such as JEDI. -CLOUDS_FORBIDDING_TELEMETRY = [] +# Add names of clouds that don't allow telemetry data collection here such as some air-gapped clouds. +CLOUDS_FORBIDDING_TELEMETRY = ['USSec', 'USNat'] # Add names of clouds that don't allow Aladdin requests for command recommendations here CLOUDS_FORBIDDING_ALADDIN_REQUEST = ['USSec', 'USNat'] From 2632e680e2c2c2f4369db54c5b3d6a9c97d215fb Mon Sep 17 00:00:00 2001 From: houk-ms Date: Mon, 14 Sep 2020 15:15:45 +0800 Subject: [PATCH 12/20] fix messages and telemetry records for extension dynamic installation --- src/azure-cli-core/azure/cli/core/parser.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/parser.py b/src/azure-cli-core/azure/cli/core/parser.py index effcbbe46b8..325982d4c2b 100644 --- a/src/azure-cli-core/azure/cli/core/parser.py +++ b/src/azure-cli-core/azure/cli/core/parser.py @@ -254,7 +254,7 @@ def has_extension_name(command_source): extension = self.command_source.extension_name # Otherwise, the command may have not been in a command group. The command source will not be # set in this case. - elif action and action.dest == '_subcommand': + elif action and action.dest in ('_subcommand', '_command_package'): # Get all parsers in the set of possible actions. parsers = list(action.choices.values()) parser = parsers[0] if parsers else None @@ -432,11 +432,15 @@ def _check_value(self, action, value): # pylint: disable=too-many-statements, t import subprocess import platform exit_code = subprocess.call(cmd_list, shell=platform.system() == 'Windows') + error_msg = ("Extension {} dynamically installed and commands will be " + "rerun automatically.").format(ext_name) + telemetry.set_user_fault(error_msg) self.exit(exit_code) else: with CommandLoggerContext(logger): - reminder = 'Extension {} installed. Please rerun your command.'.format(ext_name) - logger.error(reminder) + error_msg = 'Extension {} installed. Please rerun your command.'.format(ext_name) + logger.error(error_msg) + telemetry.set_user_fault(error_msg) self.exit(2) else: error_msg = "The command requires the extension {ext_name}. " \ @@ -454,7 +458,7 @@ def _check_value(self, action, value): # pylint: disable=too-many-statements, t candidates = difflib.get_close_matches(value, action.choices, cutoff=0.7) az_error = AzCLIError(AzCLIErrorType.ArgumentParseError, error_msg, command=self.prog) - _, params, extension = self._get_failure_recovery_arguments() + _, params, extension = self._get_failure_recovery_arguments(action) if candidates: az_error.set_recommendation("Did you mean '{}' ?".format(candidates[0])) @@ -467,7 +471,8 @@ def _check_value(self, action, value): # pylint: disable=too-many-statements, t # remind user to check extensions if we can not find a command to recommend if az_error.error_type == AzCLIErrorType.CommandNotFoundError \ - and not az_error.recommendations and self.prog == 'az': + and not az_error.recommendations and self.prog == 'az' \ + and use_dynamic_install == 'no': az_error.set_recommendation(EXTENSION_REFERENCE) az_error.set_recommendation(OVERVIEW_REFERENCE.format(command=self.prog)) From d8eb0f3ad6d285cb7f9ced0bc56cc02ce6b4da03 Mon Sep 17 00:00:00 2001 From: houk-ms Date: Mon, 14 Sep 2020 15:33:04 +0800 Subject: [PATCH 13/20] code style refining --- src/azure-cli-core/azure/cli/core/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/parser.py b/src/azure-cli-core/azure/cli/core/parser.py index 325982d4c2b..5c3c89d324d 100644 --- a/src/azure-cli-core/azure/cli/core/parser.py +++ b/src/azure-cli-core/azure/cli/core/parser.py @@ -433,7 +433,7 @@ def _check_value(self, action, value): # pylint: disable=too-many-statements, t import platform exit_code = subprocess.call(cmd_list, shell=platform.system() == 'Windows') error_msg = ("Extension {} dynamically installed and commands will be " - "rerun automatically.").format(ext_name) + "rerun automatically.").format(ext_name) telemetry.set_user_fault(error_msg) self.exit(exit_code) else: From ec6efb5822f6b28c51def7824a2ef6b30a462d6a Mon Sep 17 00:00:00 2001 From: houk-ms Date: Tue, 15 Sep 2020 13:02:43 +0800 Subject: [PATCH 14/20] disable aladdin requests for testing environment --- .azure-pipelines/templates/automation_test.yml | 1 + .../azure/cli/core/command_recommender.py | 4 +++- .../azure/cli/core/commands/__init__.py | 2 +- .../latest/test_batch_data_plane_commands.py | 18 +++++++++--------- .../tests/latest/test_network_commands.py | 2 +- .../resource/tests/latest/test_resource.py | 2 +- .../test_storage_account_scenarios.py | 4 ++++ .../latest/test_storage_account_scenarios.py | 18 +++++++++++------- .../latest/test_storage_blob_scenarios.py | 6 +++--- 9 files changed, 34 insertions(+), 23 deletions(-) diff --git a/.azure-pipelines/templates/automation_test.yml b/.azure-pipelines/templates/automation_test.yml index 76b8891b35c..8733356825a 100644 --- a/.azure-pipelines/templates/automation_test.yml +++ b/.azure-pipelines/templates/automation_test.yml @@ -21,3 +21,4 @@ steps: displayName: "Test on Profile ${{ parameters.profile }}" env: ADO_PULL_REQUEST_LATEST_COMMIT: $(System.PullRequest.TargetBranch) + DISABLE_ALADDIN_REQUEST: 'True' diff --git a/src/azure-cli-core/azure/cli/core/command_recommender.py b/src/azure-cli-core/azure/cli/core/command_recommender.py index f3ebaf75ea2..74d9944ca42 100644 --- a/src/azure-cli-core/azure/cli/core/command_recommender.py +++ b/src/azure-cli-core/azure/cli/core/command_recommender.py @@ -141,10 +141,12 @@ def recommend_a_command(self): The recommended command will be the best matched one from both the help files and the aladdin recommendations. """ + import os from azure.cli.core.cloud import CLOUDS_FORBIDDING_ALADDIN_REQUEST if self.cli_ctx and self.cli_ctx.cloud \ - and self.cli_ctx.cloud.name not in CLOUDS_FORBIDDING_ALADDIN_REQUEST: + and self.cli_ctx.cloud.name not in CLOUDS_FORBIDDING_ALADDIN_REQUEST \ + and not os.getenv('DISABLE_ALADDIN_REQUEST'): self._set_aladdin_recommendations() # all the recommended commands from help examples and aladdin diff --git a/src/azure-cli-core/azure/cli/core/commands/__init__.py b/src/azure-cli-core/azure/cli/core/commands/__init__.py index dfdae21111e..4d4e79ac646 100644 --- a/src/azure-cli-core/azure/cli/core/commands/__init__.py +++ b/src/azure-cli-core/azure/cli/core/commands/__init__.py @@ -857,7 +857,7 @@ def _validate_arg_level(self, ns, **_): # pylint: disable=no-self-use from msrest.exceptions import ValidationError if isinstance(ex, ValidationError): logger.debug('Validation error in %s.', str(validator)) - raise AzCLIError(AzCLIErrorType.ValidationError, getattr(ex, 'message', str(ex))) + raise try: delattr(ns, '_argument_validators') except AttributeError: diff --git a/src/azure-cli/azure/cli/command_modules/batch/tests/latest/test_batch_data_plane_commands.py b/src/azure-cli/azure/cli/command_modules/batch/tests/latest/test_batch_data_plane_commands.py index cd75f6c868b..fedfa0d9c3d 100644 --- a/src/azure-cli/azure/cli/command_modules/batch/tests/latest/test_batch_data_plane_commands.py +++ b/src/azure-cli/azure/cli/command_modules/batch/tests/latest/test_batch_data_plane_commands.py @@ -219,7 +219,7 @@ def test_batch_jobs_and_tasks(self, resource_group, batch_account_name): # test create job with missing parameters self.kwargs['start'] = datetime.datetime.now().isoformat() - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.batch_cmd('batch job create --id {j_id} --metadata test=value ' '--job-manager-task-environment-settings a=b ' '--job-max-task-retry-count 5 ') @@ -243,7 +243,7 @@ def test_batch_jobs_and_tasks(self, resource_group, batch_account_name): self.check('metadata[0].value', 'value')]) # test bad enum value - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.batch_cmd('batch job set --job-id {j_id} --on-all-tasks-complete badValue ') # test patch job @@ -295,26 +295,26 @@ def test_batch_pools_and_nodes( '--node-agent-sku-id "batch.node.ubuntu 16.04"') # test create pool with missing parameters - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.batch_cmd('batch pool create --id missing-params-test --os-family 4') # test create pool with missing required mutually exclusive parameters - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.batch_cmd('batch pool create --id missing-required-group-test --vm-size small') # test create pool with parameters from mutually exclusive groups - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.batch_cmd('batch pool create --id mutually-exclusive-test --vm-size small ' '--os-family 4 --image Canonical:UbuntuServer:16-LTS:latest') # test create pool with invalid vm size for IaaS - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.batch_cmd('batch pool create --id invalid-size-test --vm-size small ' '--image Canonical:UbuntuServer:16.04.0-LTS --node-agent-sku-id ' '"batch.node.ubuntu 16.04"') # test create pool with missing optional parameters - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.batch_cmd('batch pool create --id missing-optional-test --vm-size small ' '--os-family 4 --start-task-wait-for-success') @@ -326,7 +326,7 @@ def test_batch_pools_and_nodes( self.check('startTask.userIdentity.userName', 'cliTestUser')]) # test create pool from non-existant JSON file - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.batch_cmd('batch pool create --json-file batch-pool-create-missing.json') # test create pool from invalid JSON file @@ -335,7 +335,7 @@ def test_batch_pools_and_nodes( self.batch_cmd('batch pool create --json-file "{json}"') # test create pool from JSON file with additional parameters - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.kwargs['json'] = self._get_test_data_file('batch-pool-create.json').replace('\\', '\\\\') self.batch_cmd('batch pool create --json-file "{json}" --vm-size small') diff --git a/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_network_commands.py b/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_network_commands.py index a521f6ee6c7..7c36954c2d8 100644 --- a/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_network_commands.py +++ b/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_network_commands.py @@ -396,7 +396,7 @@ def test_network_public_ip_prefix(self, resource_group): ]) # Check with unsupported IP address version: IPv5 - with self.assertRaisesRegexp(Exception, '2'): + with self.assertRaisesRegexp(SystemExit, '2'): self.cmd('network public-ip prefix create -g {rg} -n {prefix_name_ipv6} --length 127 --version IPv5') diff --git a/src/azure-cli/azure/cli/command_modules/resource/tests/latest/test_resource.py b/src/azure-cli/azure/cli/command_modules/resource/tests/latest/test_resource.py index 083f04ea9bd..ce7851dba95 100644 --- a/src/azure-cli/azure/cli/command_modules/resource/tests/latest/test_resource.py +++ b/src/azure-cli/azure/cli/command_modules/resource/tests/latest/test_resource.py @@ -2582,7 +2582,7 @@ def test_resource_group_local_context(self): self.check('name', self.kwargs['group1']), self.check('location', self.kwargs['location']) ]) - with self.assertRaisesRegexp(Exception, '2'): + with self.assertRaisesRegexp(SystemExit, '2'): self.cmd('group delete') self.cmd('group delete -n {group1} -y') self.cmd('group create -n {group2}', checks=[ diff --git a/src/azure-cli/azure/cli/command_modules/storage/tests/hybrid_2019_03_01/test_storage_account_scenarios.py b/src/azure-cli/azure/cli/command_modules/storage/tests/hybrid_2019_03_01/test_storage_account_scenarios.py index 8a5cb667d31..a5aa51316e6 100644 --- a/src/azure-cli/azure/cli/command_modules/storage/tests/hybrid_2019_03_01/test_storage_account_scenarios.py +++ b/src/azure-cli/azure/cli/command_modules/storage/tests/hybrid_2019_03_01/test_storage_account_scenarios.py @@ -258,6 +258,10 @@ def test_storage_account_show_exit_codes(self, resource_group, storage_account): self.assertEqual(self.cmd('storage account show -g {rg} -n {sa}').exit_code, 0) + with self.assertRaises(SystemExit) as ex: + self.cmd('storage account show text_causing_parsing_error') + self.assertEqual(ex.exception.code, 2) + with self.assertRaises(SystemExit) as ex: self.cmd('storage account show -g fake_group -n {sa}') self.assertEqual(ex.exception.code, 3) diff --git a/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_account_scenarios.py b/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_account_scenarios.py index 0898b43c36d..6cdf8862cd8 100644 --- a/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_account_scenarios.py +++ b/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_account_scenarios.py @@ -360,7 +360,7 @@ def test_show_usage(self): self.cmd('storage account show-usage -l westus', checks=JMESPathCheck('name.value', 'StorageAccounts')) def test_show_usage_no_location(self): - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.cmd('storage account show-usage') @ResourceGroupPreparer() @@ -586,6 +586,10 @@ def test_storage_account_show_exit_codes(self, resource_group, storage_account): self.assertEqual(self.cmd('storage account show -g {rg} -n {sa}').exit_code, 0) + with self.assertRaises(SystemExit) as ex: + self.cmd('storage account show text_causing_parsing_error') + self.assertEqual(ex.exception.code, 2) + with self.assertRaises(SystemExit) as ex: self.cmd('storage account show -g fake_group -n {sa}') self.assertEqual(ex.exception.code, 3) @@ -910,22 +914,22 @@ def test_storage_account_update_delete_retention_policy(self, resource_group, st 'cmd': 'storage account blob-service-properties update' }) - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.cmd('{cmd} --enable-delete-retention true -n {sa} -g {rg}') - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.cmd('{cmd} --enable-delete-retention false --delete-retention-days 365 -n {sa} -g {rg}').get_output_in_json() - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.cmd('{cmd} --delete-retention-days 1 -n {sa} -g {rg}').get_output_in_json() - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.cmd('{cmd} --enable-delete-retention true --delete-retention-days -1 -n {sa} -g {rg}') - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.cmd('{cmd} --enable-delete-retention true --delete-retention-days 0 -n {sa} -g {rg}') - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.cmd('{cmd} --enable-delete-retention true --delete-retention-days 366 -n {sa} -g {rg}') result = self.cmd('{cmd} --enable-delete-retention true --delete-retention-days 1 -n {sa} -g {rg}').get_output_in_json() diff --git a/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_scenarios.py b/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_scenarios.py index 7bfc723e07c..c83eb487e41 100644 --- a/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_scenarios.py +++ b/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_scenarios.py @@ -522,7 +522,7 @@ def test_storage_page_blob_set_tier(self, resource_group, storage_account): self.storage_cmd('az storage blob show -c {} -n {} ', account_info, container_name, blob_name)\ .assert_with_checks(JMESPathCheck('properties.blobTier', 'P10')) - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.storage_cmd('storage blob set-tier -c {} -n {} --tier P20 -r High -t page', account_info, container_name, blob_name) @@ -546,11 +546,11 @@ def test_storage_block_blob_set_tier(self, resource_group, storage_account): self.storage_cmd('storage blob upload -c {} -n {} -f "{}"', account_info, container_name, blob_name, source_file) - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.storage_cmd('storage blob set-tier -c {} -n {} --tier Cool -r Middle', account_info, container_name, blob_name) - with self.assertRaises(Exception): + with self.assertRaises(SystemExit): self.storage_cmd('storage blob set-tier -c {} -n {} --tier Archive -r High', account_info, container_name, blob_name) From d172242c53b95754d8712bc3969408be6fed234a Mon Sep 17 00:00:00 2001 From: houk-ms Date: Tue, 15 Sep 2020 13:05:57 +0800 Subject: [PATCH 15/20] code style refining --- .../tests/hybrid_2019_03_01/test_storage_account_scenarios.py | 4 ++-- .../storage/tests/latest/test_storage_account_scenarios.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/storage/tests/hybrid_2019_03_01/test_storage_account_scenarios.py b/src/azure-cli/azure/cli/command_modules/storage/tests/hybrid_2019_03_01/test_storage_account_scenarios.py index a5aa51316e6..d9212390261 100644 --- a/src/azure-cli/azure/cli/command_modules/storage/tests/hybrid_2019_03_01/test_storage_account_scenarios.py +++ b/src/azure-cli/azure/cli/command_modules/storage/tests/hybrid_2019_03_01/test_storage_account_scenarios.py @@ -258,8 +258,8 @@ def test_storage_account_show_exit_codes(self, resource_group, storage_account): self.assertEqual(self.cmd('storage account show -g {rg} -n {sa}').exit_code, 0) - with self.assertRaises(SystemExit) as ex: - self.cmd('storage account show text_causing_parsing_error') + with self.assertRaises(SystemExit) as ex: + self.cmd('storage account show text_causing_parsing_error') self.assertEqual(ex.exception.code, 2) with self.assertRaises(SystemExit) as ex: diff --git a/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_account_scenarios.py b/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_account_scenarios.py index 6cdf8862cd8..9ef65c39e8b 100644 --- a/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_account_scenarios.py +++ b/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_account_scenarios.py @@ -586,8 +586,8 @@ def test_storage_account_show_exit_codes(self, resource_group, storage_account): self.assertEqual(self.cmd('storage account show -g {rg} -n {sa}').exit_code, 0) - with self.assertRaises(SystemExit) as ex: - self.cmd('storage account show text_causing_parsing_error') + with self.assertRaises(SystemExit) as ex: + self.cmd('storage account show text_causing_parsing_error') self.assertEqual(ex.exception.code, 2) with self.assertRaises(SystemExit) as ex: From 36278a132fe08cdca0c7a923621b2ee97af0adb8 Mon Sep 17 00:00:00 2001 From: houk-ms Date: Tue, 15 Sep 2020 13:52:31 +0800 Subject: [PATCH 16/20] add environment variable in pipeline to diable aladdin request --- azure-pipelines-full-tests.yml | 3 +++ azure-pipelines.yml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/azure-pipelines-full-tests.yml b/azure-pipelines-full-tests.yml index 029746e77e1..45a5b036b7b 100644 --- a/azure-pipelines-full-tests.yml +++ b/azure-pipelines-full-tests.yml @@ -1,6 +1,9 @@ resources: - repo: self +variables: + DISABLE_ALADDIN_REQUEST: 'True' + trigger: batch: true branches: diff --git a/azure-pipelines.yml b/azure-pipelines.yml index f1aa956161d..1cfb2315995 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -1,6 +1,9 @@ resources: - repo: self +variables: + DISABLE_ALADDIN_REQUEST: 'True' + trigger: batch: true branches: From 5110b6a49989ef99f40fe5fbd732800e9da8a52b Mon Sep 17 00:00:00 2001 From: houk-ms Date: Tue, 15 Sep 2020 14:57:16 +0800 Subject: [PATCH 17/20] remove useless environment variables --- .azure-pipelines/templates/automation_test.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.azure-pipelines/templates/automation_test.yml b/.azure-pipelines/templates/automation_test.yml index 8733356825a..76b8891b35c 100644 --- a/.azure-pipelines/templates/automation_test.yml +++ b/.azure-pipelines/templates/automation_test.yml @@ -21,4 +21,3 @@ steps: displayName: "Test on Profile ${{ parameters.profile }}" env: ADO_PULL_REQUEST_LATEST_COMMIT: $(System.PullRequest.TargetBranch) - DISABLE_ALADDIN_REQUEST: 'True' From 32c8e76dd12df4e99c0b34b8540308961bb3eb7f Mon Sep 17 00:00:00 2001 From: houk-ms Date: Tue, 15 Sep 2020 15:43:03 +0800 Subject: [PATCH 18/20] disable aladdin request for testing environments from cli core --- azure-pipelines-full-tests.yml | 3 -- azure-pipelines.yml | 3 -- .../azure/cli/core/command_recommender.py | 29 +++++++++++++++---- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/azure-pipelines-full-tests.yml b/azure-pipelines-full-tests.yml index 45a5b036b7b..029746e77e1 100644 --- a/azure-pipelines-full-tests.yml +++ b/azure-pipelines-full-tests.yml @@ -1,9 +1,6 @@ resources: - repo: self -variables: - DISABLE_ALADDIN_REQUEST: 'True' - trigger: batch: true branches: diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 1cfb2315995..f1aa956161d 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -1,9 +1,6 @@ resources: - repo: self -variables: - DISABLE_ALADDIN_REQUEST: 'True' - trigger: batch: true branches: diff --git a/src/azure-cli-core/azure/cli/core/command_recommender.py b/src/azure-cli-core/azure/cli/core/command_recommender.py index 74d9944ca42..3cb1ee5fd26 100644 --- a/src/azure-cli-core/azure/cli/core/command_recommender.py +++ b/src/azure-cli-core/azure/cli/core/command_recommender.py @@ -141,12 +141,7 @@ def recommend_a_command(self): The recommended command will be the best matched one from both the help files and the aladdin recommendations. """ - import os - from azure.cli.core.cloud import CLOUDS_FORBIDDING_ALADDIN_REQUEST - - if self.cli_ctx and self.cli_ctx.cloud \ - and self.cli_ctx.cloud.name not in CLOUDS_FORBIDDING_ALADDIN_REQUEST \ - and not os.getenv('DISABLE_ALADDIN_REQUEST'): + if not self._disable_aladdin_service(): self._set_aladdin_recommendations() # all the recommended commands from help examples and aladdin @@ -177,6 +172,28 @@ def recommend_a_command(self): return recommend_command + def _disable_aladdin_service(self): + """Decide whether to disable aladdin request when a command fails. + The possible cases to disable it are: + 1. In air-gapped clouds + 2. In testing environments + """ + from azure.cli.core.cloud import CLOUDS_FORBIDDING_ALADDIN_REQUEST + + # CLI is not started well + if not self.cli_ctx or not self.cli_ctx.cloud: + return True + + # for air-gapped clouds + if self.cli_ctx.cloud.name in CLOUDS_FORBIDDING_ALADDIN_REQUEST: + return True + + # for testing environments + if self.cli_ctx.__class__.__name__ == 'DummyCli': + return True + + return False + def _get_parameter_list(self, raw_command): # pylint: disable=no-self-use """Get the paramter list from a raw command string An example: 'az group create -n test -l eastus' ==> ['-n', '-l'] From cbce73cf139fdb4c656ca22940d4becdeed1411f Mon Sep 17 00:00:00 2001 From: houk-ms Date: Tue, 15 Sep 2020 17:13:36 +0800 Subject: [PATCH 19/20] send errorneous command names to Aladdin --- .../azure/cli/core/command_recommender.py | 50 ++++++++++--------- src/azure-cli-core/azure/cli/core/parser.py | 4 +- 2 files changed, 29 insertions(+), 25 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/command_recommender.py b/src/azure-cli-core/azure/cli/core/command_recommender.py index 3cb1ee5fd26..e7c2a69d492 100644 --- a/src/azure-cli-core/azure/cli/core/command_recommender.py +++ b/src/azure-cli-core/azure/cli/core/command_recommender.py @@ -42,7 +42,7 @@ class CommandRecommender(): def __init__(self, command, parameters, extension, error_msg, cli_ctx): """ - :param command: The command name in user's input, which should be corrected if misspelled. + :param command: The command name in user's input. :type command: str :param parameters: The parameter arguments in users input. :type parameters: list @@ -144,29 +144,33 @@ def recommend_a_command(self): if not self._disable_aladdin_service(): self._set_aladdin_recommendations() - # all the recommended commands from help examples and aladdin - all_commands = self.help_examples + self.aladdin_recommendations - all_commands.sort(key=len) - - filtered_commands = [] - filtered_choices = [] - target = ''.join(self.parameters) - - for command in all_commands: - # filter out the commands which begins with a different command group - if command.startswith('az {}'.format(self.command)): - parameters = self._get_parameter_list(command) - normalized_parameters = self._normalize_parameters(parameters) - filtered_choices.append(''.join(normalized_parameters)) - filtered_commands.append(command) - - # sort the commands by argument matches - candidates = difflib.get_close_matches(target, filtered_choices, cutoff=0) - recommend_command = '' - if candidates: - index = filtered_choices.index(candidates[0]) - recommend_command = filtered_commands[index] + if self.help_examples and self.aladdin_recommendations: + # all the recommended commands from help examples and aladdin + all_commands = self.help_examples + self.aladdin_recommendations + all_commands.sort(key=len) + + filtered_commands = [] + filtered_choices = [] + target = ''.join(self.parameters) + example_command_name = self.help_examples[0].split(' -')[0] + + for command in all_commands: + # keep only the commands which begin with a same command name with examples + if command.startswith(example_command_name): + parameters = self._get_parameter_list(command) + normalized_parameters = self._normalize_parameters(parameters) + filtered_choices.append(''.join(normalized_parameters)) + filtered_commands.append(command) + + # sort the commands by argument matches + candidates = difflib.get_close_matches(target, filtered_choices, cutoff=0) + + if candidates: + index = filtered_choices.index(candidates[0]) + recommend_command = filtered_commands[index] + + # fallback to use the first recommended command from Aladdin elif self.aladdin_recommendations: recommend_command = self.aladdin_recommendations[0] diff --git a/src/azure-cli-core/azure/cli/core/parser.py b/src/azure-cli-core/azure/cli/core/parser.py index 5c3c89d324d..6b2d4d1c853 100644 --- a/src/azure-cli-core/azure/cli/core/parser.py +++ b/src/azure-cli-core/azure/cli/core/parser.py @@ -458,12 +458,12 @@ def _check_value(self, action, value): # pylint: disable=too-many-statements, t candidates = difflib.get_close_matches(value, action.choices, cutoff=0.7) az_error = AzCLIError(AzCLIErrorType.ArgumentParseError, error_msg, command=self.prog) - _, params, extension = self._get_failure_recovery_arguments(action) + command_arguments = self._get_failure_recovery_arguments(action) if candidates: az_error.set_recommendation("Did you mean '{}' ?".format(candidates[0])) # recommand a command for user - recommender = CommandRecommender(command_name_inferred[3:].strip(), params, extension, error_msg, cli_ctx) + recommender = CommandRecommender(*command_arguments, error_msg, cli_ctx) recommender.set_help_examples(self.get_examples(command_name_inferred)) recommended_command = recommender.recommend_a_command() if recommended_command: From a94c6f757fd6be4b28e64574ba1440edf381cc3d Mon Sep 17 00:00:00 2001 From: houk-ms Date: Tue, 15 Sep 2020 17:35:57 +0800 Subject: [PATCH 20/20] fix test parser --- src/azure-cli-core/azure/cli/core/tests/test_parser.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/tests/test_parser.py b/src/azure-cli-core/azure/cli/core/tests/test_parser.py index 0507ed2c58f..3f295db6c68 100644 --- a/src/azure-cli-core/azure/cli/core/tests/test_parser.py +++ b/src/azure-cli-core/azure/cli/core/tests/test_parser.py @@ -245,7 +245,6 @@ def mock_add_extension(*args, **kwargs): # assert the right choices are matched as "close". # If these don't hold, matching algorithm should be deemed flawed. - choice_lists = [choice_lists[index] for index in range(len(choice_lists)) if not index % 2] for choices in choice_lists[:2]: self.assertEqual(len(choices), 1) self.assertEqual(len(choice_lists[2]), 0)