From f11784d3652f9fa9f7a19aa6a4405ed4a579b456 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Mon, 15 Mar 2021 14:50:29 +0800 Subject: [PATCH 1/3] =?UTF-8?q?=EF=BB=BFnot=20minify?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cli/command_modules/feedback/custom.py | 258 +++--------------- 1 file changed, 32 insertions(+), 226 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/feedback/custom.py b/src/azure-cli/azure/cli/command_modules/feedback/custom.py index e125ec4c73e..4d9fe62fdaf 100644 --- a/src/azure-cli/azure/cli/command_modules/feedback/custom.py +++ b/src/azure-cli/azure/cli/command_modules/feedback/custom.py @@ -3,29 +3,22 @@ # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- -from __future__ import print_function -import os -import re +import datetime import math +import os import platform -import datetime - -try: - from urllib.parse import urlencode # python 3 -except ImportError: - from urllib import urlencode # python 2 - from collections import namedtuple +from urllib.parse import urlencode + +from azure.cli.core.azlogging import _UNKNOWN_COMMAND, _CMD_LOG_LINE_PREFIX +from azure.cli.core.commands.constants import SURVEY_PROMPT +from azure.cli.core.extension._resolve import resolve_project_url_from_index, NoExtensionCandidatesError +from azure.cli.core.util import get_az_version_string, open_page_in_browser, can_launch_browser, in_cloud_console from knack.log import get_logger from knack.prompting import prompt, NoTTYException from knack.util import CLIError -from azure.cli.core.extension._resolve import resolve_project_url_from_index, NoExtensionCandidatesError -from azure.cli.core.util import get_az_version_string, open_page_in_browser, can_launch_browser, in_cloud_console -from azure.cli.core.azlogging import _UNKNOWN_COMMAND, _CMD_LOG_LINE_PREFIX -from azure.cli.core.commands.constants import SURVEY_PROMPT - _ONE_MIN_IN_SECS = 60 _ONE_HR_IN_SECS = 3600 @@ -58,18 +51,14 @@ _MSG_ISSUE = "Would you like to create an issue? Enter Y or N: " -_ISSUES_TEMPLATE_PREFIX = """ - -BEGIN TEMPLATE -=============== -**If possible, a browser will be opened to {} to create an issue.** -**You can also run `az feedback --verbose` to emit the full issue draft to stderr.** -**Azure CLI repo: {}** -**Azure CLI Extensions repo: {}** +_OPEN_BROWSER_INSTRUCTION = """ +* If possible, a browser will be opened to {} to create an issue. +* If the URL length exceeds GitHub or the browser's limitation and causes the content to be trimmed, you can run `az feedback --verbose` to emit the full issue draft to stderr. +* Azure CLI repo: {} +* Azure CLI Extensions repo: {} """ _ISSUES_TEMPLATE = """ - ### **This is autogenerated. Please review and update as needed.** ## Describe the bug @@ -78,7 +67,9 @@ `{command_name}` **Errors:** +``` {errors_string} +``` ## To Reproduce: Steps to reproduce the behavior. Note that argument values have been redacted, as they may contain sensitive information. @@ -349,160 +340,6 @@ def _get_info_from_log_line(line, p_id): return CommandLogFile._LogRecordType(*parts) -class ErrorMinifier: - - _FILE_RE = re.compile(r'File "(.*)"') - _CONTINUATION_STR = "...\n" - - def __init__(self, errors_list): - self._errors_list = errors_list - self._capacity = None # to how many symbols should minify - self._minified_error = "\n".join(self._errors_list) - - def set_capacity(self, capacity): - logger.debug("Capacity for error string: %s", capacity) - - self._capacity = int(capacity) - self._minified_error = self._get_minified_errors() - - def _get_minified_errors(self): # pylint: disable=too-many-return-statements - errors_list = self._errors_list - errors_string = "\n".join(errors_list) - if self._capacity is None: - return errors_string - - if not errors_list: - return "" - - # if within capacity return string - if len(errors_string) <= self._capacity: - return errors_string - - # shorten file names and try again - for i, error in enumerate(errors_list): - errors_list[i] = self._minify_by_shortening_file_names(error, levels=5) - errors_string = "\n".join(errors_list) - if len(errors_string) <= self._capacity: - return errors_string - - # shorten file names and try again - for i, error in enumerate(errors_list): - errors_list[i] = self._minify_by_shortening_file_names(error, levels=4) - errors_string = "\n".join(errors_list) - if len(errors_string) <= self._capacity: - return errors_string - - # return first exception if multiple exceptions occurs - for i, error in enumerate(errors_list): - errors_list[i] = self._minify_by_removing_nested_exceptions(error) - errors_string = "\n".join(errors_list) - if len(errors_string) <= self._capacity: - return errors_string - - # remove some lines - errors_string = self._minify_by_removing_lines(errors_string, desired_length=self._capacity) - if len(errors_string) <= self._capacity: - return errors_string - - # last resort: strip off the suffix - return self._minify_by_removing_suffix(errors_string, desired_length=self._capacity) - - @staticmethod - def _minify_by_shortening_file_names(error_string, levels=5): - new_lines = [] - for line in error_string.splitlines(): - # if original exception - if line.strip().startswith("File") and ", line" in line: - parts = line.split(",") - match = ErrorMinifier._FILE_RE.search(parts[0]) - if match: - parts[0] = ErrorMinifier._shorten_file_name(match.group(1), levels) - parts[1] = parts[1].replace("line", "ln") - line = ",".join(parts) - # if cleaned exceptions - elif ".py" in line and ", ln" in line: - parts = line.split(",") - parts[0] = ErrorMinifier._shorten_file_name(parts[0], levels) - line = ",".join(parts) - # remove this line - elif "here is the traceback" in line.lower(): - continue - - new_lines.append(line) - - return "\n".join(new_lines) - - @staticmethod - def _shorten_file_name(file_name, levels=5): - if levels > 0: - new_name = os.path.basename(file_name) - file_name = os.path.dirname(file_name) - for _ in range(levels - 1): - new_name = os.path.join(os.path.basename(file_name), new_name) - file_name = os.path.dirname(file_name) - return new_name - return file_name - - @staticmethod - def _minify_by_removing_nested_exceptions(error_string): - lines = error_string.splitlines() - - idx = len(lines) - 1 - for i, line in enumerate(lines): - if "During handling of the above exception" in line: - idx = i - break - - # if unchanged return error_string - if idx == len(lines) - 1: - return error_string - - lines = lines[:idx] + [ErrorMinifier._CONTINUATION_STR] + lines[-3:] - return "\n".join(lines) - - @staticmethod - def _minify_by_removing_lines(error_string, desired_length): - """ - Repeatedly remove the lines from the middle, as a last resort remove even the - first line but keep the last one, in the effort to strip down the error string - to the desired length. - """ - if len(error_string) <= desired_length: - return error_string - - symbols_to_remove = len(error_string) - desired_length + len(ErrorMinifier._CONTINUATION_STR) - lines = error_string.splitlines(keepends=True) - - if len(lines) <= 1 or symbols_to_remove <= 0: - # nothing to remove - return error_string - - mid = 0 - while len(lines) > 1 and symbols_to_remove > 0: - # remove the middle line, when even prefer to remove the one closer to the start - mid = (len(lines) - 1) // 2 - symbols_to_remove -= len(lines.pop(mid)) - - lines.insert(mid, ErrorMinifier._CONTINUATION_STR) - return "".join(lines) - - @staticmethod - def _minify_by_removing_suffix(error_string, desired_length): - """ - Strip off the suffix of the error string, force it to be <= desired length. - """ - if len(error_string) <= desired_length: - return error_string - - continuation = ErrorMinifier._CONTINUATION_STR.strip()[:desired_length] - return error_string[:desired_length - len(continuation)] + continuation - - def __str__(self): - if self._minified_error: - return "```\n{}\n```".format(self._minified_error.strip()) - return "" - - def _build_issue_info_tup(command_log_file=None): format_dict = {"command_name": "", "errors_string": "", "executed_command": ""} @@ -526,7 +363,7 @@ def _build_issue_info_tup(command_log_file=None): is_ext = True ext_name = extension_name - format_dict["errors_string"] = ErrorMinifier(errors_list) + format_dict["errors_string"] = ''.join(errors_list) format_dict["executed_command"] = "az " + executed_command if executed_command else executed_command format_dict["command_name"] += extension_info @@ -542,35 +379,13 @@ def _build_issue_info_tup(command_log_file=None): pretty_url_name = _get_extension_repo_url(ext_name) if is_ext else _CLI_ISSUES_URL # get issue body without minification - original_issue_body = _ISSUES_TEMPLATE.format(**format_dict) - - # First try - capacity = _MAX_URL_LENGTH # some browsers support a max of roughly 2000 characters - res = _get_minified_issue_url(command_log_file, format_dict.copy(), is_ext, ext_name, capacity) - formatted_issues_url, minified_issue_body = res - capacity = capacity - (len(formatted_issues_url) - _MAX_URL_LENGTH) - - # while formatted issue to long, minify to new capacity - tries = 0 - while len(formatted_issues_url) > _MAX_URL_LENGTH and tries < 25: - # reduce capacity by difference if formatted_issues_url is too long because of url escaping - res = _get_minified_issue_url(command_log_file, format_dict.copy(), is_ext, ext_name, capacity) - formatted_issues_url, minified_issue_body = res - capacity = capacity - (len(formatted_issues_url) - _MAX_URL_LENGTH) - tries += 1 - - # if something went wrong with minification (i.e. another part of the issue is unexpectedly too long) - # then truncate the whole issue body and warn the user. - if len(formatted_issues_url) > _MAX_URL_LENGTH: - formatted_issues_url = formatted_issues_url[:_MAX_URL_LENGTH] - logger.warning("Failed to properly minify issue url. " - "Please use 'az feedback --verbose' to get the full issue output.") - - logger.debug("Total minified issue length is %s", len(minified_issue_body)) + issue_body = _ISSUES_TEMPLATE.format(**format_dict) + formatted_issues_url = _get_issue_url(command_log_file, format_dict.copy(), is_ext, ext_name) + logger.debug("Total formatted url length is %s", len(formatted_issues_url)) - return _ISSUES_TEMPLATE_PREFIX.format(pretty_url_name, _CLI_ISSUES_URL, _EXTENSIONS_ISSUES_URL), \ - formatted_issues_url, original_issue_body + return _OPEN_BROWSER_INSTRUCTION.format(pretty_url_name, _CLI_ISSUES_URL, _EXTENSIONS_ISSUES_URL), \ + formatted_issues_url, issue_body def _get_extension_repo_url(ext_name, raw=False): @@ -597,32 +412,22 @@ def _is_valid_github_project_url(url): return False -def _get_minified_issue_url(command_log_file, format_dict, is_ext, ext_name, capacity): - # get issue body without errors - minified_errors = format_dict["errors_string"] - format_dict["errors_string"] = "" - no_errors_issue_body = _ISSUES_TEMPLATE.format(**format_dict) - - # get minified issue body - format_dict["errors_string"] = minified_errors - if hasattr(minified_errors, "set_capacity"): - logger.debug("Length of issue body before errors added: %s", len(no_errors_issue_body)) - minified_errors.set_capacity( - capacity - len(no_errors_issue_body)) # factor in length of url and expansion of url escaped characters - minified_issue_body = _ISSUES_TEMPLATE.format(**format_dict) +def _get_issue_url(command_log_file, format_dict, is_ext, ext_name): + issue_body = _ISSUES_TEMPLATE.format(**format_dict) # prefix formatted url with 'https://' if necessary and supply empty body to remove any existing issue template # aka.ms doesn't work well for long urls / query params formatted_issues_url = _get_extension_repo_url(ext_name, raw=True) if is_ext else _RAW_CLI_ISSUES_URL if not formatted_issues_url.startswith("http"): formatted_issues_url = "https://" + formatted_issues_url - query_dict = {'body': minified_issue_body} + # https://docs.github.com/en/github/managing-your-work-on-github/about-automation-for-issues-and-pull-requests-with-query-parameters + query_dict = {'body': issue_body} if command_log_file and command_log_file.failed(): query_dict['template'] = 'Bug_report.md' new_placeholder = urlencode(query_dict) formatted_issues_url = "{}?{}".format(formatted_issues_url, new_placeholder) - return formatted_issues_url, minified_issue_body + return formatted_issues_url def _get_az_version_summary(): @@ -749,19 +554,20 @@ def _prompt_issue(recent_command_list): if ans in ["y", "n"]: if ans == "y": - prefix, url, original_issue = _build_issue_info_tup() + browser_instruction, url, issue_body = _build_issue_info_tup() else: return False else: if ans in ["q", "quit"]: return False if ans == 0: - prefix, url, original_issue = _build_issue_info_tup() + browser_instruction, url, issue_body = _build_issue_info_tup() else: - prefix, url, original_issue = _build_issue_info_tup(recent_command_list[ans]) - print(prefix) + browser_instruction, url, issue_body = _build_issue_info_tup(recent_command_list[ans]) + + logger.info(issue_body) + print(browser_instruction) - logger.info(original_issue) # if we are not in cloud shell and can launch a browser, launch it with the issue draft if can_launch_browser() and not in_cloud_console(): open_page_in_browser(url) From 94cdb4470c708863511c0c06ed0d340f6d7f8f8e Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Wed, 7 Apr 2021 17:07:10 +0800 Subject: [PATCH 2/3] Format issue_body only once --- src/azure-cli/azure/cli/command_modules/feedback/custom.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/feedback/custom.py b/src/azure-cli/azure/cli/command_modules/feedback/custom.py index 4d9fe62fdaf..b417f680e00 100644 --- a/src/azure-cli/azure/cli/command_modules/feedback/custom.py +++ b/src/azure-cli/azure/cli/command_modules/feedback/custom.py @@ -380,7 +380,7 @@ def _build_issue_info_tup(command_log_file=None): # get issue body without minification issue_body = _ISSUES_TEMPLATE.format(**format_dict) - formatted_issues_url = _get_issue_url(command_log_file, format_dict.copy(), is_ext, ext_name) + formatted_issues_url = _get_issue_url(command_log_file, issue_body, is_ext, ext_name) logger.debug("Total formatted url length is %s", len(formatted_issues_url)) @@ -412,9 +412,7 @@ def _is_valid_github_project_url(url): return False -def _get_issue_url(command_log_file, format_dict, is_ext, ext_name): - issue_body = _ISSUES_TEMPLATE.format(**format_dict) - +def _get_issue_url(command_log_file, issue_body, is_ext, ext_name): # prefix formatted url with 'https://' if necessary and supply empty body to remove any existing issue template # aka.ms doesn't work well for long urls / query params formatted_issues_url = _get_extension_repo_url(ext_name, raw=True) if is_ext else _RAW_CLI_ISSUES_URL From 7e9104776483c1430338058effb4d0418bca555c Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Wed, 7 Apr 2021 17:09:54 +0800 Subject: [PATCH 3/3] Remove comment --- src/azure-cli/azure/cli/command_modules/feedback/custom.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/feedback/custom.py b/src/azure-cli/azure/cli/command_modules/feedback/custom.py index 75d52883431..1b2b36b79ce 100644 --- a/src/azure-cli/azure/cli/command_modules/feedback/custom.py +++ b/src/azure-cli/azure/cli/command_modules/feedback/custom.py @@ -377,7 +377,6 @@ def _build_issue_info_tup(command_log_file=None): pretty_url_name = _get_extension_repo_url(ext_name) if is_ext else _CLI_ISSUES_URL - # get issue body without minification issue_body = _ISSUES_TEMPLATE.format(**format_dict) formatted_issues_url = _get_issue_url(command_log_file, issue_body, is_ext, ext_name)