From 782c1708832830868625b38b8b9c9dc9eafe7b9f Mon Sep 17 00:00:00 2001 From: zmeir Date: Sun, 20 Aug 2023 09:23:21 +0300 Subject: [PATCH 1/5] Support custom deployments for github_app.py and add more options for automatic review actions --- pr_agent/servers/github_app.py | 101 ++++++++++++++++++++------- pr_agent/settings/configuration.toml | 13 ++++ 2 files changed, 90 insertions(+), 24 deletions(-) diff --git a/pr_agent/servers/github_app.py b/pr_agent/servers/github_app.py index 498bb81f..8bd0fd4d 100644 --- a/pr_agent/servers/github_app.py +++ b/pr_agent/servers/github_app.py @@ -1,6 +1,8 @@ import copy import logging import sys +import os +import time from typing import Any, Dict import uvicorn @@ -14,7 +16,7 @@ from pr_agent.config_loader import get_settings, global_settings from pr_agent.git_providers import get_git_provider from pr_agent.servers.utils import verify_signature -logging.basicConfig(stream=sys.stdout, level=logging.DEBUG) +logging.basicConfig(stream=sys.stdout, level=logging.INFO) router = APIRouter() @@ -34,7 +36,8 @@ async def handle_github_webhooks(request: Request, response: Response): context["installation_id"] = installation_id context["settings"] = copy.deepcopy(global_settings) - return await handle_request(body) + response = await handle_request(body, event=request.headers.get("X-GitHub-Event", None)) + return response or {} @router.post("/api/v1/marketplace_webhooks") @@ -48,70 +51,120 @@ async def get_body(request): except Exception as e: logging.error("Error parsing request body", e) raise HTTPException(status_code=400, detail="Error parsing request body") from e - body_bytes = await request.body() - signature_header = request.headers.get('x-hub-signature-256', None) - webhook_secret = getattr(get_settings().github, 'webhook_secret', None) - if webhook_secret: - verify_signature(body_bytes, webhook_secret, signature_header) + if get_settings().github_app.verify_signature: + body_bytes = await request.body() + signature_header = request.headers.get('x-hub-signature-256', None) + webhook_secret = getattr(get_settings().github, 'webhook_secret', None) + if webhook_secret: + verify_signature(body_bytes, webhook_secret, signature_header) return body +_duplicate_requests_cache = {} -async def handle_request(body: Dict[str, Any]): +async def handle_request(body: Dict[str, Any], event: str): """ Handle incoming GitHub webhook requests. Args: body: The request body. + event: The GitHub event type. """ - action = body.get("action") + action = body.get("action", None) if not action: return {} agent = PRAgent() + bot_user = get_settings().github_app.bot_user + logging.info(f"action: '{action}'") + logging.info(f"event: '{event}'") + if get_settings().github_app.duplicate_requests_cache and _is_duplicate_request(body): + return {} + + # handle all sorts of comment events (e.g. issue_comment) if action == 'created': if "comment" not in body: return {} - comment_body = body.get("comment", {}).get("body") - sender = body.get("sender", {}).get("login") - if sender and 'bot' in sender: + comment_body = body.get("comment", {}).get("body", None) + sender = body.get("sender", {}).get("login", None) + if sender and bot_user in sender: + logging.info(f"Ignoring comment from {bot_user} user") return {} - if "issue" not in body or "pull_request" not in body["issue"]: + logging.info(f"Processing comment from {sender} user") + if "issue" in body and "pull_request" in body["issue"] and "url" in body["issue"]["pull_request"]: + api_url = body["issue"]["pull_request"]["url"] + elif "comment" in body and "pull_request_url" in body["comment"]: + api_url = body["comment"]["pull_request_url"] + else: return {} - pull_request = body["issue"]["pull_request"] - api_url = pull_request.get("url") + logging.info(f"Handling comment because of event={event} and action={action}") comment_id = body.get("comment", {}).get("id") provider = get_git_provider()(pr_url=api_url) await agent.handle_request(api_url, comment_body, notify=lambda: provider.add_eyes_reaction(comment_id)) - - elif action == "opened" or 'reopened' in action: - pull_request = body.get("pull_request") + # handle pull_request event: + # automatically review opened/reopened/ready_for_review PRs as long as they're not in draft, + # as well as direct review requests from the bot + elif event == 'pull_request': + pull_request = body.get("pull_request", None) if not pull_request: return {} - api_url = pull_request.get("url") - if not api_url: + api_url = pull_request.get("url", None) + if api_url is None: return {} - await agent.handle_request(api_url, "/auto_review") + if pull_request.get("draft", True) or pull_request.get("state", None) != "open" or pull_request.get("user", {}).get("login", "") == bot_user: + return {} + if action in get_settings().github_app.handle_pr_actions: + if action == "review_requested": + if body.get("requested_reviewer", {}).get("login", "") != bot_user: + return {} + if pull_request.get("created_at", None) == pull_request.get("updated_at", None): + # avoid double reviews when opening a PR for the first time + return {} + logging.info(f"Performing review because of event={event} and action={action}") + for command in get_settings().github_app.pr_commands: + logging.info(f"Performing command: {command}") + await agent.handle_request(api_url, command) + logging.info("event or action does not require handling") return {} +def _is_duplicate_request(body: Dict[str, Any]) -> bool: + """ + In some deployments its possible to get duplicate requests if the handling is long, + This function checks if the request is duplicate and if so - ignores it. + """ + request_hash = hash(str(body)) + logging.info(f"request_hash: {request_hash}") + request_time = time.monotonic() + ttl = get_settings().github_app.duplicate_requests_cache_ttl # in seconds + to_delete = [key for key, key_time in _duplicate_requests_cache.items() if request_time - key_time > ttl] + for key in to_delete: + del _duplicate_requests_cache[key] + is_duplicate = request_hash in _duplicate_requests_cache + _duplicate_requests_cache[request_hash] = request_time + if is_duplicate: + logging.info(f"Ignoring duplicate request {request_hash}") + return is_duplicate + + @router.get("/") async def root(): return {"status": "ok"} def start(): - # Override the deployment type to app - get_settings().set("GITHUB.DEPLOYMENT_TYPE", "app") + if get_settings().github_app.override_deployment_type: + # Override the deployment type to app + get_settings().set("GITHUB.DEPLOYMENT_TYPE", "app") get_settings().set("CONFIG.PUBLISH_OUTPUT_PROGRESS", False) middleware = [Middleware(RawContextMiddleware)] app = FastAPI(middleware=middleware) app.include_router(router) - uvicorn.run(app, host="0.0.0.0", port=3000) + uvicorn.run(app, host="0.0.0.0", port=int(os.environ.get("PORT", "3000"))) if __name__ == '__main__': diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index ce920efd..5658d6e2 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -43,6 +43,19 @@ extra_instructions = "" deployment_type = "user" ratelimit_retries = 5 +[github_app] +# these toggles allows running the github app from custom deployments +bot_user = "github-actions[bot]" +verify_signature = true +override_deployment_type = true +# in some deployments it's possible to get duplicate requests if the handling is long, +# these settings are used to avoid handling duplicate requests. +duplicate_requests_cache = false +duplicate_requests_cache_ttl = 60 # in seconds +# settings for "pull_request" event +handle_pr_actions = ['opened', 'reopened', 'ready_for_review', 'review_requested'] +pr_commands = ["/describe", "/auto_review"] + [gitlab] # URL to the gitlab service url = "https://gitlab.com" From 62fe1de12d4dd3b6fb3e48693e7e98ab617f7e07 Mon Sep 17 00:00:00 2001 From: Zohar Meir <33152084+zmeir@users.noreply.github.com> Date: Tue, 22 Aug 2023 18:28:06 +0300 Subject: [PATCH 2/5] Remove redundant toggle --- pr_agent/servers/github_app.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pr_agent/servers/github_app.py b/pr_agent/servers/github_app.py index 8bd0fd4d..a80daa03 100644 --- a/pr_agent/servers/github_app.py +++ b/pr_agent/servers/github_app.py @@ -51,12 +51,11 @@ async def get_body(request): except Exception as e: logging.error("Error parsing request body", e) raise HTTPException(status_code=400, detail="Error parsing request body") from e - if get_settings().github_app.verify_signature: + webhook_secret = getattr(get_settings().github, 'webhook_secret', None) + if webhook_secret: body_bytes = await request.body() signature_header = request.headers.get('x-hub-signature-256', None) - webhook_secret = getattr(get_settings().github, 'webhook_secret', None) - if webhook_secret: - verify_signature(body_bytes, webhook_secret, signature_header) + verify_signature(body_bytes, webhook_secret, signature_header) return body From a9a7a55f0246861b5a1dc1e52d0931d3c45a52fb Mon Sep 17 00:00:00 2001 From: Zohar Meir <33152084+zmeir@users.noreply.github.com> Date: Tue, 22 Aug 2023 18:28:43 +0300 Subject: [PATCH 3/5] Remove redundant toggle --- pr_agent/settings/configuration.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index 5658d6e2..68db675c 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -46,7 +46,6 @@ ratelimit_retries = 5 [github_app] # these toggles allows running the github app from custom deployments bot_user = "github-actions[bot]" -verify_signature = true override_deployment_type = true # in some deployments it's possible to get duplicate requests if the handling is long, # these settings are used to avoid handling duplicate requests. From 3d771e28ce72a7cd4476d62e2956a0e39267eec1 Mon Sep 17 00:00:00 2001 From: Zohar Meir <33152084+zmeir@users.noreply.github.com> Date: Tue, 22 Aug 2023 18:33:25 +0300 Subject: [PATCH 4/5] Remove redundant None default in dict.get --- pr_agent/servers/github_app.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pr_agent/servers/github_app.py b/pr_agent/servers/github_app.py index a80daa03..7a7208d0 100644 --- a/pr_agent/servers/github_app.py +++ b/pr_agent/servers/github_app.py @@ -70,7 +70,7 @@ async def handle_request(body: Dict[str, Any], event: str): body: The request body. event: The GitHub event type. """ - action = body.get("action", None) + action = body.get("action") if not action: return {} agent = PRAgent() @@ -85,8 +85,8 @@ async def handle_request(body: Dict[str, Any], event: str): if action == 'created': if "comment" not in body: return {} - comment_body = body.get("comment", {}).get("body", None) - sender = body.get("sender", {}).get("login", None) + comment_body = body.get("comment", {}).get("body") + sender = body.get("sender", {}).get("login") if sender and bot_user in sender: logging.info(f"Ignoring comment from {bot_user} user") return {} @@ -106,19 +106,19 @@ async def handle_request(body: Dict[str, Any], event: str): # automatically review opened/reopened/ready_for_review PRs as long as they're not in draft, # as well as direct review requests from the bot elif event == 'pull_request': - pull_request = body.get("pull_request", None) + pull_request = body.get("pull_request") if not pull_request: return {} - api_url = pull_request.get("url", None) - if api_url is None: + api_url = pull_request.get("url") + if not api_url: return {} - if pull_request.get("draft", True) or pull_request.get("state", None) != "open" or pull_request.get("user", {}).get("login", "") == bot_user: + if pull_request.get("draft", True) or pull_request.get("state") != "open" or pull_request.get("user", {}).get("login", "") == bot_user: return {} if action in get_settings().github_app.handle_pr_actions: if action == "review_requested": if body.get("requested_reviewer", {}).get("login", "") != bot_user: return {} - if pull_request.get("created_at", None) == pull_request.get("updated_at", None): + if pull_request.get("created_at") == pull_request.get("updated_at"): # avoid double reviews when opening a PR for the first time return {} logging.info(f"Performing review because of event={event} and action={action}") From f17b4fcc9eec44220c26e4fa13aa1c7c57356590 Mon Sep 17 00:00:00 2001 From: zmeir Date: Tue, 22 Aug 2023 21:14:03 +0300 Subject: [PATCH 5/5] Made the automatic describe command the least destructive --- pr_agent/settings/configuration.toml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index b763ef2e..c0cadf58 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -61,7 +61,10 @@ duplicate_requests_cache = false duplicate_requests_cache_ttl = 60 # in seconds # settings for "pull_request" event handle_pr_actions = ['opened', 'reopened', 'ready_for_review', 'review_requested'] -pr_commands = ["/describe", "/auto_review"] +pr_commands = [ + "/describe --pr_description.add_original_user_description=true --pr_description.keep_original_user_title=true", + "/auto_review", +] [gitlab] # URL to the gitlab service