From 5c4bc0a00821b103970ac37e0e3de13437eb899d Mon Sep 17 00:00:00 2001 From: mrT23 Date: Mon, 12 Aug 2024 09:48:26 +0300 Subject: [PATCH] Add Bitbucket diff handling and improve error logging - Implement `publish_file_comments` method placeholder - Enhance `is_supported` method to include `publish_file_comments` - Refactor diff splitting logic to handle Bitbucket-specific headers - Improve error handling and logging for file content retrieval - Add `get_pr_owner_id` method to retrieve PR owner ID - Update `_get_pr_file_content` to fetch file content from remote link - Fix variable name typo in `extend_patch` function in `git_patch_processing.py` --- pr_agent/algo/git_patch_processing.py | 6 +-- pr_agent/git_providers/bitbucket_provider.py | 56 ++++++++++++++++---- 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/pr_agent/algo/git_patch_processing.py b/pr_agent/algo/git_patch_processing.py index cfe5e9369..00f16d7bc 100644 --- a/pr_agent/algo/git_patch_processing.py +++ b/pr_agent/algo/git_patch_processing.py @@ -8,7 +8,7 @@ def extend_patch(original_file_str, patch_str, patch_extra_lines_before=0, patch_extra_lines_after=0) -> str: - if not patch_str or (patch_extra_lines_before == 0 and patch_extra_lines_after == 0): + if not patch_str or (patch_extra_lines_before == 0 and patch_extra_lines_after == 0) or not original_file_str: return patch_str if type(original_file_str) == bytes: @@ -52,9 +52,9 @@ def extend_patch(original_file_str, patch_str, patch_extra_lines_before=0, patch extended_size1 = size1 + (start1 - extended_start1) + patch_extra_lines_after extended_start2 = max(1, start2 - patch_extra_lines_before) extended_size2 = size2 + (start2 - extended_start2) + patch_extra_lines_after - if extended_start1 - 1 + extended_size1 > len(original_lines): + if extended_start1 - 1 + extended_size1 > len_original_lines: # we cannot extend beyond the original file - delta_cap = extended_start1 - 1 + extended_size1 - len(original_lines) + delta_cap = extended_start1 - 1 + extended_size1 - len_original_lines extended_size1 = max(extended_size1 - delta_cap, size1) extended_size2 = max(extended_size2 - delta_cap, size2) delta_lines = original_lines[extended_start1 - 1:start1 - 1] diff --git a/pr_agent/git_providers/bitbucket_provider.py b/pr_agent/git_providers/bitbucket_provider.py index a29e25bef..4759a585f 100644 --- a/pr_agent/git_providers/bitbucket_provider.py +++ b/pr_agent/git_providers/bitbucket_provider.py @@ -108,8 +108,12 @@ def publish_code_suggestions(self, code_suggestions: list) -> bool: get_logger().error(f"Failed to publish code suggestion, error: {e}") return False + def publish_file_comments(self, file_comments: list) -> bool: + pass + def is_supported(self, capability: str) -> bool: - if capability in ['get_issue_comments', 'publish_inline_comments', 'get_labels', 'gfm_markdown']: + if capability in ['get_issue_comments', 'publish_inline_comments', 'get_labels', 'gfm_markdown', + 'publish_file_comments']: return False return True @@ -137,9 +141,28 @@ def get_diff_files(self) -> list[FilePatchInfo]: except Exception as e: pass - diff_split = [ - "diff --git%s" % x for x in self.pr.diff().split("diff --git") if x.strip() - ] + # get the pr patches + pr_patch = self.pr.diff() + diff_split = ["diff --git" + x for x in pr_patch.split("diff --git") if x.strip()] + if len(diff_split) != len(diffs): + get_logger().error(f"Error - failed to split the diff into {len(diffs)} parts") + return [] + # bitbucket diff has a header for each file, we need to remove it: + # "diff --git filename + # index caa56f0..61528d7 100644 + # --- a/pr_agent/cli_pip.py + # +++ b/pr_agent/cli_pip.py + # @@ -... @@" + for i, _ in enumerate(diff_split): + diff_split_lines = diff_split[i].splitlines() + if (len(diff_split_lines) > 5 and + diff_split_lines[2].startswith("---") and + diff_split_lines[3].startswith("+++") and + diff_split_lines[4].startswith("@@")): + diff_split[i] = "\n".join(diff_split_lines[4:]) + else: + get_logger().error(f"Error - failed to remove the bitbucket header from diff {i}") + break invalid_files_names = [] diff_files = [] @@ -148,10 +171,14 @@ def get_diff_files(self) -> list[FilePatchInfo]: invalid_files_names.append(diff.new.path) continue - original_file_content_str = self._get_pr_file_content( - diff.old.get_data("links") - ) - new_file_content_str = self._get_pr_file_content(diff.new.get_data("links")) + try: + original_file_content_str = self._get_pr_file_content(diff.old.get_data("links")['self']['href']) + new_file_content_str = self._get_pr_file_content(diff.new.get_data("links")['self']['href']) + except Exception as e: + get_logger().exception(f"Error - bitbucket failed to get file content, error: {e}") + original_file_content_str = "" + new_file_content_str = "" + file_patch_canonic_structure = FilePatchInfo( original_file_content_str, new_file_content_str, @@ -172,7 +199,6 @@ def get_diff_files(self) -> list[FilePatchInfo]: if invalid_files_names: get_logger().info(f"Disregarding files with invalid extensions:\n{invalid_files_names}") - self.diff_files = diff_files return diff_files @@ -314,6 +340,9 @@ def get_languages(self): def get_pr_branch(self): return self.pr.source_branch + def get_pr_owner_id(self) -> str | None: + return self.workspace_slug + def get_pr_description_full(self): return self.pr.description @@ -400,7 +429,14 @@ def create_or_update_pr_file(self, file_path: str, branch: str, contents="", mes get_logger().exception(f"Failed to create empty file {file_path} in branch {branch}") def _get_pr_file_content(self, remote_link: str): - return "" + try: + response = requests.request("GET", remote_link, headers=self.headers) + if response.status_code == 404: # not found + return "" + contents = response.text + return contents + except Exception: + return "" def get_commit_messages(self): return "" # not implemented yet