From c8bca487e5e750d805532c4e5cfbc4f5819d3e0a Mon Sep 17 00:00:00 2001 From: Sagi Medina Date: Mon, 8 Jan 2024 08:59:12 +0200 Subject: [PATCH 1/2] feat: Implement methods in AzureDevopsProvider for publishing code suggestions, labels, and removing comments --- .../git_providers/azuredevops_provider.py | 382 +++++++++++++----- 1 file changed, 292 insertions(+), 90 deletions(-) diff --git a/pr_agent/git_providers/azuredevops_provider.py b/pr_agent/git_providers/azuredevops_provider.py index 0552a63e..58bd2d6e 100644 --- a/pr_agent/git_providers/azuredevops_provider.py +++ b/pr_agent/git_providers/azuredevops_provider.py @@ -1,29 +1,147 @@ -import json +import os from typing import Optional, Tuple from urllib.parse import urlparse -import os - from ..log import get_logger AZURE_DEVOPS_AVAILABLE = True try: from msrest.authentication import BasicAuthentication from azure.devops.connection import Connection - from azure.devops.v7_1.git.models import Comment, CommentThread, GitVersionDescriptor, GitPullRequest -except ImportError: + from azure.devops.v7_1.git.models import ( + Comment, + CommentThread, + GitVersionDescriptor, + GitPullRequest, + ) +except ImportError as e: AZURE_DEVOPS_AVAILABLE = False -from ..config_loader import get_settings -from ..algo.utils import load_large_diff, clip_tokens from ..algo.language_handler import is_valid_file -from .git_provider import EDIT_TYPE, FilePatchInfo +from ..algo.utils import clip_tokens, load_large_diff +from ..config_loader import get_settings +from .git_provider import EDIT_TYPE, FilePatchInfo, GitProvider -class AzureDevopsProvider: - def __init__(self, pr_url: Optional[str] = None, incremental: Optional[bool] = False): +class AzureDevopsProvider(GitProvider): + def publish_code_suggestions(self, code_suggestions: list) -> bool: + """ + Publishes code suggestions as comments on the PR. + """ + post_parameters_list = [] + for suggestion in code_suggestions: + body = suggestion['body'] + relevant_file = suggestion['relevant_file'] + relevant_lines_start = suggestion['relevant_lines_start'] + relevant_lines_end = suggestion['relevant_lines_end'] + + if not relevant_lines_start or relevant_lines_start == -1: + if get_settings().config.verbosity_level >= 2: + get_logger().exception( + f"Failed to publish code suggestion, relevant_lines_start is {relevant_lines_start}") + continue + + if relevant_lines_end < relevant_lines_start: + if get_settings().config.verbosity_level >= 2: + get_logger().exception(f"Failed to publish code suggestion, " + f"relevant_lines_end is {relevant_lines_end} and " + f"relevant_lines_start is {relevant_lines_start}") + continue + + if relevant_lines_end > relevant_lines_start: + post_parameters = { + "body": body, + "path": relevant_file, + "line": relevant_lines_end, + "start_line": relevant_lines_start, + "start_side": "RIGHT", + } + else: # API is different for single line comments + post_parameters = { + "body": body, + "path": relevant_file, + "line": relevant_lines_start, + "side": "RIGHT", + } + post_parameters_list.append(post_parameters) + + try: + for post_parameters in post_parameters_list: + comment = Comment(content=post_parameters["body"], comment_type=1) + thread = CommentThread(comments=[comment], + thread_context={ + "filePath": post_parameters["path"], + "rightFileStart": { + "line": post_parameters["start_line"], + "offset": 1, + }, + "rightFileEnd": { + "line": post_parameters["line"], + "offset": 1, + }, + }) + self.azure_devops_client.create_thread( + comment_thread=thread, + project=self.workspace_slug, + repository_id=self.repo_slug, + pull_request_id=self.pr_num + ) + if get_settings().config.verbosity_level >= 2: + get_logger().info( + f"Published code suggestion on {self.pr_num} at {post_parameters['path']}" + ) + return True + except Exception as e: + if get_settings().config.verbosity_level >= 2: + get_logger().error(f"Failed to publish code suggestion, error: {e}") + return False + + def get_pr_description_full(self) -> str: + pass + + def remove_comment(self, comment): + try: + self.azure_devops_client.delete_comment( + repository_id=self.repo_slug, + pull_request_id=self.pr_num, + thread_id=comment["thread_id"], + comment_id=comment["comment_id"], + project=self.workspace_slug, + ) + except Exception as e: + get_logger().exception(f"Failed to remove comment, error: {e}") + + def publish_labels(self, pr_types): + try: + for pr_type in pr_types: + self.azure_devops_client.create_pull_request_label( + label={"name": pr_type}, + project=self.workspace_slug, + repository_id=self.repo_slug, + pull_request_id=self.pr_num, + ) + except Exception as e: + get_logger().exception(f"Failed to publish labels, error: {e}") + + def get_pr_labels(self): + try: + labels = self.azure_devops_client.get_pull_request_labels( + project=self.workspace_slug, + repository_id=self.repo_slug, + pull_request_id=self.pr_num, + ) + return [label.name for label in labels] + except Exception as e: + get_logger().exception(f"Failed to get labels, error: {e}") + return [] + + def __init__( + self, pr_url: Optional[str] = None, incremental: Optional[bool] = False + ): if not AZURE_DEVOPS_AVAILABLE: - raise ImportError("Azure DevOps provider is not available. Please install the required dependencies.") + raise ImportError( + "Azure DevOps provider is not available. Please install the required dependencies." + ) self.azure_devops_client = self._get_azure_devops_client() @@ -38,8 +156,11 @@ class AzureDevopsProvider: self.set_pr(pr_url) def is_supported(self, capability: str) -> bool: - if capability in ['get_issue_comments', 'create_inline_comment', 'publish_inline_comments', 'get_labels', - 'remove_initial_comment', 'gfm_markdown']: + if capability in [ + "get_issue_comments", + "create_inline_comment", + "publish_inline_comments", + ]: return False return True @@ -49,10 +170,14 @@ class AzureDevopsProvider: def get_repo_settings(self): try: - contents = self.azure_devops_client.get_item_content(repository_id=self.repo_slug, - project=self.workspace_slug, download=False, - include_content_metadata=False, include_content=True, - path=".pr_agent.toml") + contents = self.azure_devops_client.get_item_content( + repository_id=self.repo_slug, + project=self.workspace_slug, + download=False, + include_content_metadata=False, + include_content=True, + path=".pr_agent.toml", + ) return contents except Exception as e: get_logger().exception("get repo settings error") @@ -60,15 +185,19 @@ class AzureDevopsProvider: def get_files(self): files = [] - for i in self.azure_devops_client.get_pull_request_commits(project=self.workspace_slug, - repository_id=self.repo_slug, - pull_request_id=self.pr_num): - - changes_obj = self.azure_devops_client.get_changes(project=self.workspace_slug, - repository_id=self.repo_slug, commit_id=i.commit_id) + for i in self.azure_devops_client.get_pull_request_commits( + project=self.workspace_slug, + repository_id=self.repo_slug, + pull_request_id=self.pr_num, + ): + changes_obj = self.azure_devops_client.get_changes( + project=self.workspace_slug, + repository_id=self.repo_slug, + commit_id=i.commit_id, + ) for c in changes_obj.changes: - files.append(c['item']['path']) + files.append(c["item"]["path"]) return list(set(files)) def get_diff_files(self) -> list[FilePatchInfo]: @@ -76,22 +205,27 @@ class AzureDevopsProvider: base_sha = self.pr.last_merge_target_commit head_sha = self.pr.last_merge_source_commit - commits = self.azure_devops_client.get_pull_request_commits(project=self.workspace_slug, - repository_id=self.repo_slug, - pull_request_id=self.pr_num) + commits = self.azure_devops_client.get_pull_request_commits( + project=self.workspace_slug, + repository_id=self.repo_slug, + pull_request_id=self.pr_num, + ) diff_files = [] diffs = [] diff_types = {} for c in commits: - changes_obj = self.azure_devops_client.get_changes(project=self.workspace_slug, - repository_id=self.repo_slug, commit_id=c.commit_id) + changes_obj = self.azure_devops_client.get_changes( + project=self.workspace_slug, + repository_id=self.repo_slug, + commit_id=c.commit_id, + ) for i in changes_obj.changes: - if(i['item']['gitObjectType'] == 'tree'): + if i["item"]["gitObjectType"] == "tree": continue - diffs.append(i['item']['path']) - diff_types[i['item']['path']] = i['changeType'] + diffs.append(i["item"]["path"]) + diff_types[i["item"]["path"]] = i["changeType"] diffs = list(set(diffs)) @@ -99,47 +233,72 @@ class AzureDevopsProvider: if not is_valid_file(file): continue - version = GitVersionDescriptor(version=head_sha.commit_id, version_type='commit') + version = GitVersionDescriptor( + version=head_sha.commit_id, version_type="commit" + ) try: - new_file_content_str = self.azure_devops_client.get_item(repository_id=self.repo_slug, - path=file, - project=self.workspace_slug, - version_descriptor=version, - download=False, - include_content=True) + new_file_content_str = self.azure_devops_client.get_item( + repository_id=self.repo_slug, + path=file, + project=self.workspace_slug, + version_descriptor=version, + download=False, + include_content=True, + ) new_file_content_str = new_file_content_str.content except Exception as error: - get_logger().error("Failed to retrieve new file content of %s at version %s. Error: %s", file, version, str(error)) + get_logger().error( + "Failed to retrieve new file content of %s at version %s. Error: %s", + file, + version, + str(error), + ) new_file_content_str = "" edit_type = EDIT_TYPE.MODIFIED - if diff_types[file] == 'add': + if diff_types[file] == "add": edit_type = EDIT_TYPE.ADDED - elif diff_types[file] == 'delete': + elif diff_types[file] == "delete": edit_type = EDIT_TYPE.DELETED - elif diff_types[file] == 'rename': + elif diff_types[file] == "rename": edit_type = EDIT_TYPE.RENAMED - version = GitVersionDescriptor(version=base_sha.commit_id, version_type='commit') + version = GitVersionDescriptor( + version=base_sha.commit_id, version_type="commit" + ) try: - original_file_content_str = self.azure_devops_client.get_item(repository_id=self.repo_slug, - path=file, - project=self.workspace_slug, - version_descriptor=version, - download=False, - include_content=True) + original_file_content_str = self.azure_devops_client.get_item( + repository_id=self.repo_slug, + path=file, + project=self.workspace_slug, + version_descriptor=version, + download=False, + include_content=True, + ) original_file_content_str = original_file_content_str.content except Exception as error: - get_logger().error("Failed to retrieve original file content of %s at version %s. Error: %s", file, version, str(error)) + get_logger().error( + "Failed to retrieve original file content of %s at version %s. Error: %s", + file, + version, + str(error), + ) original_file_content_str = "" - patch = load_large_diff(file, new_file_content_str, original_file_content_str) + patch = load_large_diff( + file, new_file_content_str, original_file_content_str + ) - diff_files.append(FilePatchInfo(original_file_content_str, new_file_content_str, - patch=patch, - filename=file, - edit_type=edit_type)) + diff_files.append( + FilePatchInfo( + original_file_content_str, + new_file_content_str, + patch=patch, + filename=file, + edit_type=edit_type, + ) + ) self.diff_files = diff_files return diff_files @@ -150,64 +309,92 @@ class AzureDevopsProvider: def publish_comment(self, pr_comment: str, is_temporary: bool = False): comment = Comment(content=pr_comment) thread = CommentThread(comments=[comment]) - thread_response = self.azure_devops_client.create_thread(comment_thread=thread, project=self.workspace_slug, - repository_id=self.repo_slug, - pull_request_id=self.pr_num) + thread_response = self.azure_devops_client.create_thread( + comment_thread=thread, + project=self.workspace_slug, + repository_id=self.repo_slug, + pull_request_id=self.pr_num, + ) if is_temporary: - self.temp_comments.append({'thread_id': thread_response.id, 'comment_id': comment.id}) + self.temp_comments.append( + {"thread_id": thread_response.id, "comment_id": thread_response.comments[0].id} + ) def publish_description(self, pr_title: str, pr_body: str): try: updated_pr = GitPullRequest() updated_pr.title = pr_title updated_pr.description = pr_body - self.azure_devops_client.update_pull_request(project=self.workspace_slug, - repository_id=self.repo_slug, - pull_request_id=self.pr_num, - git_pull_request_to_update=updated_pr) + self.azure_devops_client.update_pull_request( + project=self.workspace_slug, + repository_id=self.repo_slug, + pull_request_id=self.pr_num, + git_pull_request_to_update=updated_pr, + ) except Exception as e: - get_logger().exception(f"Could not update pull request {self.pr_num} description: {e}") + get_logger().exception( + f"Could not update pull request {self.pr_num} description: {e}" + ) def remove_initial_comment(self): - return "" # not implemented yet + try: + for comment in self.temp_comments: + self.remove_comment(comment) + except Exception as e: + get_logger().exception(f"Failed to remove temp comments, error: {e}") - def publish_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): - raise NotImplementedError("Azure DevOps provider does not support publishing inline comment yet") + def publish_inline_comment( + self, body: str, relevant_file: str, relevant_line_in_file: str + ): + raise NotImplementedError( + "Azure DevOps provider does not support publishing inline comment yet" + ) def publish_inline_comments(self, comments: list[dict]): - raise NotImplementedError("Azure DevOps provider does not support publishing inline comments yet") + raise NotImplementedError( + "Azure DevOps provider does not support publishing inline comments yet" + ) def get_title(self): return self.pr.title def get_languages(self): languages = [] - files = self.azure_devops_client.get_items(project=self.workspace_slug, repository_id=self.repo_slug, - recursion_level="Full", include_content_metadata=True, - include_links=False, download=False) + files = self.azure_devops_client.get_items( + project=self.workspace_slug, + repository_id=self.repo_slug, + recursion_level="Full", + include_content_metadata=True, + include_links=False, + download=False, + ) for f in files: - if f.git_object_type == 'blob': + if f.git_object_type == "blob": file_name, file_extension = os.path.splitext(f.path) languages.append(file_extension[1:]) extension_counts = {} for ext in languages: - if ext != '': + if ext != "": extension_counts[ext] = extension_counts.get(ext, 0) + 1 total_extensions = sum(extension_counts.values()) - extension_percentages = {ext: (count / total_extensions) * 100 for ext, count in extension_counts.items()} + extension_percentages = { + ext: (count / total_extensions) * 100 + for ext, count in extension_counts.items() + } return extension_percentages def get_pr_branch(self): - pr_info = self.azure_devops_client.get_pull_request_by_id(project=self.workspace_slug, - pull_request_id=self.pr_num) - source_branch = pr_info.source_ref_name.split('/')[-1] + pr_info = self.azure_devops_client.get_pull_request_by_id( + project=self.workspace_slug, pull_request_id=self.pr_num + ) + source_branch = pr_info.source_ref_name.split("/")[-1] return source_branch - def get_pr_description(self): + def get_pr_description(self, full=False): max_tokens = get_settings().get("CONFIG.MAX_DESCRIPTION_TOKENS", None) if max_tokens: return clip_tokens(self.pr.description, max_tokens) @@ -217,7 +404,9 @@ class AzureDevopsProvider: return 0 def get_issue_comments(self): - raise NotImplementedError("Azure DevOps provider does not support issue comments yet") + raise NotImplementedError( + "Azure DevOps provider does not support issue comments yet" + ) def add_eyes_reaction(self, issue_comment_id: int) -> Optional[int]: return True @@ -226,16 +415,20 @@ class AzureDevopsProvider: return True def get_issue_comments(self): - raise NotImplementedError("Azure DevOps provider does not support issue comments yet") + raise NotImplementedError( + "Azure DevOps provider does not support issue comments yet" + ) @staticmethod def _parse_pr_url(pr_url: str) -> Tuple[str, int]: parsed_url = urlparse(pr_url) - path_parts = parsed_url.path.strip('/').split('/') + path_parts = parsed_url.path.strip("/").split("/") - if len(path_parts) < 6 or path_parts[4] != 'pullrequest': - raise ValueError("The provided URL does not appear to be a Azure DevOps PR URL") + if len(path_parts) < 6 or path_parts[4] != "pullrequest": + raise ValueError( + "The provided URL does not appear to be a Azure DevOps PR URL" + ) workspace_slug = path_parts[1] repo_slug = path_parts[3] @@ -251,10 +444,9 @@ class AzureDevopsProvider: pat = get_settings().azure_devops.pat org = get_settings().azure_devops.org except AttributeError as e: - raise ValueError( - "Azure DevOps PAT token is required ") from e + raise ValueError("Azure DevOps PAT token is required ") from e - credentials = BasicAuthentication('', pat) + credentials = BasicAuthentication("", pat) azure_devops_connection = Connection(base_url=org, creds=credentials) azure_devops_client = azure_devops_connection.clients.get_git_client() @@ -262,13 +454,23 @@ class AzureDevopsProvider: def _get_repo(self): if self.repo is None: - self.repo = self.azure_devops_client.get_repository(project=self.workspace_slug, - repository_id=self.repo_slug) + self.repo = self.azure_devops_client.get_repository( + project=self.workspace_slug, repository_id=self.repo_slug + ) return self.repo def _get_pr(self): - self.pr = self.azure_devops_client.get_pull_request_by_id(pull_request_id=self.pr_num, project=self.workspace_slug) + self.pr = self.azure_devops_client.get_pull_request_by_id( + pull_request_id=self.pr_num, project=self.workspace_slug + ) return self.pr def get_commit_messages(self): return "" # not implemented yet + + def get_pr_id(self): + try: + pr_id = f"{self.workspace_slug}/{self.repo_slug}/{self.pr_num}" + return pr_id + except Exception: + return "" From b776e5069c148ed5699a5c4fc462945555851f25 Mon Sep 17 00:00:00 2001 From: Sagi Medina Date: Mon, 8 Jan 2024 09:15:34 +0200 Subject: [PATCH 2/2] feat: Refactor AzureDevopsProvider class in azuredevops_provider.py - Reorder class methods and constructor for better readability - Add error logging for failed operations - Implement get_pr_description_full method - Update get_pr_description method to always return full description - Modify _parse_pr_url method to return workspace_slug, repo_slug, and pr_number - Make _get_azure_devops_client a static method - Add error handling in get_pr_id method --- .../git_providers/azuredevops_provider.py | 78 ++++++++++--------- 1 file changed, 40 insertions(+), 38 deletions(-) diff --git a/pr_agent/git_providers/azuredevops_provider.py b/pr_agent/git_providers/azuredevops_provider.py index 58bd2d6e..17d87488 100644 --- a/pr_agent/git_providers/azuredevops_provider.py +++ b/pr_agent/git_providers/azuredevops_provider.py @@ -3,27 +3,51 @@ from typing import Optional, Tuple from urllib.parse import urlparse from ..log import get_logger +from ..algo.language_handler import is_valid_file +from ..algo.utils import clip_tokens, load_large_diff +from ..config_loader import get_settings +from .git_provider import EDIT_TYPE, FilePatchInfo, GitProvider AZURE_DEVOPS_AVAILABLE = True + try: + # noinspection PyUnresolvedReferences from msrest.authentication import BasicAuthentication + # noinspection PyUnresolvedReferences from azure.devops.connection import Connection + # noinspection PyUnresolvedReferences from azure.devops.v7_1.git.models import ( Comment, CommentThread, GitVersionDescriptor, GitPullRequest, ) -except ImportError as e: +except ImportError: AZURE_DEVOPS_AVAILABLE = False -from ..algo.language_handler import is_valid_file -from ..algo.utils import clip_tokens, load_large_diff -from ..config_loader import get_settings -from .git_provider import EDIT_TYPE, FilePatchInfo, GitProvider - class AzureDevopsProvider(GitProvider): + + def __init__( + self, pr_url: Optional[str] = None, incremental: Optional[bool] = False + ): + if not AZURE_DEVOPS_AVAILABLE: + raise ImportError( + "Azure DevOps provider is not available. Please install the required dependencies." + ) + + self.azure_devops_client = self._get_azure_devops_client() + + self.workspace_slug = None + self.repo_slug = None + self.repo = None + self.pr_num = None + self.pr = None + self.temp_comments = [] + self.incremental = incremental + if pr_url: + self.set_pr(pr_url) + def publish_code_suggestions(self, code_suggestions: list) -> bool: """ Publishes code suggestions as comments on the PR. @@ -97,7 +121,7 @@ class AzureDevopsProvider(GitProvider): return False def get_pr_description_full(self) -> str: - pass + return self.pr.description def remove_comment(self, comment): try: @@ -135,26 +159,6 @@ class AzureDevopsProvider(GitProvider): get_logger().exception(f"Failed to get labels, error: {e}") return [] - def __init__( - self, pr_url: Optional[str] = None, incremental: Optional[bool] = False - ): - if not AZURE_DEVOPS_AVAILABLE: - raise ImportError( - "Azure DevOps provider is not available. Please install the required dependencies." - ) - - self.azure_devops_client = self._get_azure_devops_client() - - self.workspace_slug = None - self.repo_slug = None - self.repo = None - self.pr_num = None - self.pr = None - self.temp_comments = [] - self.incremental = incremental - if pr_url: - self.set_pr(pr_url) - def is_supported(self, capability: str) -> bool: if capability in [ "get_issue_comments", @@ -180,7 +184,8 @@ class AzureDevopsProvider(GitProvider): ) return contents except Exception as e: - get_logger().exception("get repo settings error") + if get_settings().config.verbosity_level >= 2: + get_logger().error(f"Failed to get repo settings, error: {e}") return "" def get_files(self): @@ -300,7 +305,6 @@ class AzureDevopsProvider(GitProvider): ) ) - self.diff_files = diff_files return diff_files except Exception as e: print(f"Error: {str(e)}") @@ -394,7 +398,7 @@ class AzureDevopsProvider(GitProvider): source_branch = pr_info.source_ref_name.split("/")[-1] return source_branch - def get_pr_description(self, full=False): + def get_pr_description(self, *, full: bool = True) -> str: max_tokens = get_settings().get("CONFIG.MAX_DESCRIPTION_TOKENS", None) if max_tokens: return clip_tokens(self.pr.description, max_tokens) @@ -414,13 +418,8 @@ class AzureDevopsProvider(GitProvider): def remove_reaction(self, issue_comment_id: int, reaction_id: int) -> bool: return True - def get_issue_comments(self): - raise NotImplementedError( - "Azure DevOps provider does not support issue comments yet" - ) - @staticmethod - def _parse_pr_url(pr_url: str) -> Tuple[str, int]: + def _parse_pr_url(pr_url: str) -> Tuple[str, str, int]: parsed_url = urlparse(pr_url) path_parts = parsed_url.path.strip("/").split("/") @@ -439,7 +438,8 @@ class AzureDevopsProvider(GitProvider): return workspace_slug, repo_slug, pr_number - def _get_azure_devops_client(self): + @staticmethod + def _get_azure_devops_client(): try: pat = get_settings().azure_devops.pat org = get_settings().azure_devops.org @@ -472,5 +472,7 @@ class AzureDevopsProvider(GitProvider): try: pr_id = f"{self.workspace_slug}/{self.repo_slug}/{self.pr_num}" return pr_id - except Exception: + except Exception as e: + if get_settings().config.verbosity_level >= 2: + get_logger().error(f"Failed to get pr id, error: {e}") return ""