From 40e754e569e9aad0183712b96b3da37de04a961f Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Thu, 18 Jan 2024 19:49:44 -0500 Subject: [PATCH 1/2] Reuse auth headers to do requests --- .../clang_tidy_review/__init__.py | 71 ++++++------------- post/clang_tidy_review/pyproject.toml | 1 - 2 files changed, 20 insertions(+), 52 deletions(-) diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py index bc2e87d..52dcde5 100644 --- a/post/clang_tidy_review/clang_tidy_review/__init__.py +++ b/post/clang_tidy_review/clang_tidy_review/__init__.py @@ -12,7 +12,6 @@ from operator import itemgetter import pprint import pathlib -import requests import subprocess import textwrap import unidiff @@ -23,6 +22,7 @@ import io import zipfile from github import Github, Auth +from github.GithubException import GithubException from github.Requester import Requester from github.PaginatedList import PaginatedList from github.WorkflowRun import WorkflowRun @@ -247,26 +247,15 @@ def pull_request(self): self._pull_request = self.repo.get_pull(int(self.pr_number)) return self._pull_request - def headers(self, media_type: str): - return { - "Accept": f"application/vnd.github.{media_type}", - "Authorization": f"token {self.token}", - } - - @property - def base_url(self): - return f"{self.api_url}/repos/{self.repo_name}/pulls/{self.pr_number}" - - def get(self, media_type: str, extra: str = "") -> str: - url = f"{self.base_url}{extra}" - response = requests.get(url, headers=self.headers(media_type)) - response.raise_for_status() - return response.text - def get_pr_diff(self) -> List[unidiff.PatchSet]: """Download the PR diff, return a list of PatchedFile""" - diffs = self.get("v3.diff") + _, data = self.repo._requester.requestJsonAndCheck( + "GET", + self.pull_request.url, + headers={"Accept": f"application/vnd.github.{'v3.diff'}"}, + ) + diffs = data["data"] # PatchSet is the easiest way to construct what we want, but the # diff_line_no property on lines is counted from the top of the @@ -292,7 +281,7 @@ def get_element( return PaginatedList( get_element, self.pull_request._requester, - f"{self.base_url}/comments", + self.pull_request.review_comments_url, None, ) @@ -311,33 +300,9 @@ def post_lgtm_comment(self, body: str): self.pull_request.create_issue_comment(body) - def post_review(self, review): + def post_review(self, review: PRReview): """Submit a completed review""" - headers = { - "Accept": "application/vnd.github.comfort-fade-preview+json", - "Authorization": f"token {self.token}", - } - url = f"{self.base_url}/reviews" - - post_review_response = requests.post(url, json=review, headers=headers) - print(post_review_response.text) - try: - post_review_response.raise_for_status() - except requests.exceptions.HTTPError as e: - if e.response.status_code == 403: - print( - "::error title=Missing permissions::This workflow does not have " - "enough permissions to submit a review. This could be because " - "the GitHub token specified for this workflow is invalid or " - "missing permissions, or it could be because this pull request " - "comes from a fork which reduces the default token permissions. " - "To support forked workflows, see the " - "https://github.com/ZedThree/clang-tidy-review#usage-in-fork-environments " - "instructions" - ) - - # Re-raise the exception, causing an error in the workflow - raise e + self.pull_request.create_review(**review) def post_annotations(self, review): headers = { @@ -346,8 +311,9 @@ def post_annotations(self, review): } url = f"{self.api_url}/repos/{self.repo_name}/check-runs" - response = requests.post(url, json=review, headers=headers) - response.raise_for_status() + self.repo._requester.requestJsonAndCheck( + "POST", url, parameters=review, headers=headers + ) @contextlib.contextmanager @@ -920,14 +886,17 @@ def download_artifacts(pull: PullRequest, workflow_id: int): ) return None, None - r = requests.get(artifact.archive_download_url, headers=pull.headers("json")) - if not r.ok: + try: + _, data = pull.repo._requester.requestJsonAndCheck( + "GET", artifact.archive_download_url, headers=pull.headers("json") + ) + except GithubException as exc: print( - f"WARNING: Couldn't automatically download artifacts for workflow '{workflow_id}', response was: {r}: {r.reason}" + f"WARNING: Couldn't automatically download artifacts for workflow '{workflow_id}', response was: {exc}" ) return None, None - contents = b"".join(r.iter_content()) + contents = b"".join(data["data"].iter_content()) data = zipfile.ZipFile(io.BytesIO(contents)) filenames = data.namelist() diff --git a/post/clang_tidy_review/pyproject.toml b/post/clang_tidy_review/pyproject.toml index f1d9056..01ba865 100644 --- a/post/clang_tidy_review/pyproject.toml +++ b/post/clang_tidy_review/pyproject.toml @@ -15,7 +15,6 @@ license = {text = "MIT"} dependencies = [ "PyGithub ~= 2.1", "unidiff ~= 0.6.0", - "requests ~= 2.23", "pyyaml ~= 6.0.1", ] keywords = ["C++", "static-analysis"] From baadf424d2af3b418e4c7de25989570fded55e34 Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Fri, 19 Jan 2024 16:32:56 -0500 Subject: [PATCH 2/2] Use set difference to cull comments --- .../clang_tidy_review/__init__.py | 91 ++++++++++--------- 1 file changed, 48 insertions(+), 43 deletions(-) diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py index 52dcde5..b0f2d61 100644 --- a/post/clang_tidy_review/clang_tidy_review/__init__.py +++ b/post/clang_tidy_review/clang_tidy_review/__init__.py @@ -60,6 +60,44 @@ class PRReview(TypedDict): comments: List[PRReviewComment] +class HashableComment: + def __init__(self, body: str, line: int, path: str, side: str, **kwargs): + self.body = body + self.line = line + self.path = path + self.side = side + + def __hash__(self): + return hash( + ( + self.body, + self.line, + self.path, + self.side, + ) + ) + + def __eq__(self, other): + return ( + type(self) is type(other) + and self.body == other.body + and self.line == self.line + and other.path == other.path + and self.side == other.side + ) + + def __lt__(self, other): + if self.path != other.path: + return self.path < other.path + if self.line != other.line: + return self.line < other.line + if self.side != other.side: + return self.side < other.side + if self.body != other.body: + return self.body < other.body + return id(self) < id(other) + + def add_auth_arguments(parser: argparse.ArgumentParser): # Token parser.add_argument("--token", help="github auth token") @@ -952,39 +990,11 @@ def load_and_merge_reviews(review_files: List[pathlib.Path]) -> Optional[PRRevie result = reviews[0] - class Comment: - def __init__(self, data): - self.data = data - - def __hash__(self): - return hash( - ( - self.data["body"], - self.data["line"], - self.data["path"], - self.data["side"], - ) - ) - - def __eq__(self, other): - return type(other) is Comment and self.data == other.data - - def __lt__(self, other): - if self.data["path"] != other.data["path"]: - return self.data["path"] < other.data["path"] - if self.data["line"] != other.data["line"]: - return self.data["line"] < other.data["line"] - if self.data["side"] != other.data["side"]: - return self.data["side"] < other.data["side"] - if self.data["body"] != other.data["body"]: - return self.data["body"] < other.data["body"] - return hash(self) < hash(other) - comments = set() for review in reviews: - comments.update(map(Comment, review["comments"])) + comments.update(map(lambda c: HashableComment(**c), review["comments"])) - result["comments"] = [c.data for c in sorted(comments)] + result["comments"] = [c.__dict__ for c in sorted(comments)] return result @@ -1032,19 +1042,14 @@ def cull_comments(pull_request: PullRequest, review, max_comments): """ - comments = pull_request.get_pr_comments() - - for comment in comments: - review["comments"] = list( - filter( - lambda review_comment: not ( - review_comment["path"] == comment["path"] - and review_comment["line"] == comment["line"] - and review_comment["body"] == comment["body"] - ), - review["comments"], - ) - ) + unposted_comments = set(map(lambda c: HashableComment(**c), review["comments"])) + posted_comments = set( + map(lambda c: HashableComment(**c), pull_request.get_pr_comments()) + ) + + review["comments"] = [ + c.__dict__ for c in sorted(unposted_comments - posted_comments) + ] if len(review["comments"]) > max_comments: review["body"] += (