From 22d17985a1df61f705edd28cad1d906f2e271d14 Mon Sep 17 00:00:00 2001 From: zmeir Date: Sun, 7 Jan 2024 16:00:44 +0200 Subject: [PATCH] Less noisy fallback for `publish_code_suggestions` in case of invalid comments As a first option, `publish_code_suggestions` will try to post all review comments in a single GitHub review. This is preferred because it will group all comments together in the GitHub UI under the same review, and will trigger just one notification for any viewers of the PR. If just one of the comments is malformed, the entire API request will fail and none of the comments will be posted to the PR. In the current implementation, the fallback mechanism is to just post each comment separately with `try/except` and skip the invalid comments. This works, but potentially creates a lot of noise in the PR as each comment is posted as in a separate review, creating multiple notifications. This suggested fallback is based on a similar idea, but without creating multiple review notifications. The it works is by iterating over the potential comments, and starting a PENDING review for the current comment. The review is not submitted and does not trigger a notification, but it is verified against the GitHub API, and so we can verify if the comment is valid. After checking all comments we then submit a single review with all the verified comments which is guaranteed to succeed. The end result is having the exact same comments posted to the PR as with the current fallback method, but the downside is having twice as many API calls (for each comment we have 1 extra API call to delete the pending review). --- pr_agent/git_providers/github_provider.py | 35 ++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index a6ffbcdf..0ca5e1ff 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -271,7 +271,40 @@ class GithubProvider(GitProvider): except Exception as e: if get_settings().config.verbosity_level >= 2: get_logger().error(f"Failed to publish code suggestion, error: {e}") - return False + if getattr(e, "status", None) == 422 and getattr(e, "data", {}).get("message", None) == "Unprocessable Entity": + pass # trying to find the bad comment + else: + return False + try: + import time + verified_comments = [] + invalid_comments = [] + for comment in post_parameters_list: + time.sleep(1) # for avoiding secondary rate limit + try: + headers, data = self.pr._requester.requestJsonAndCheck( + "POST", f"{self.pr.url}/reviews", input=dict(commit_id=self.last_commit_id.sha, comments=[comment]) + ) + pending_review_id = data["id"] + verified_comments.append(comment) + except Exception as e: + invalid_comments.append((comment, e)) + pending_review_id = None + if pending_review_id is not None: + try: + self.pr._requester.requestJsonAndCheck("DELETE", f"{self.pr.url}/reviews/{pending_review_id}") + except Exception as e: + pass + if verified_comments: + self.pr.create_review(commit=self.last_commit_id, comments=verified_comments) + if invalid_comments: + if get_settings().config.verbosity_level >= 2: + get_logger().error(f"Invalid comments: {invalid_comments}") + return True + except Exception as e: + if get_settings().config.verbosity_level >= 2: + get_logger().error(f"Failed to publish code suggestion fallback, error: {e}") + return False def remove_initial_comment(self): try: