From be555cdf38b9278ab905421946689a93bf452ecc Mon Sep 17 00:00:00 2001 From: driazati Date: Thu, 13 Jan 2022 14:37:50 -0800 Subject: [PATCH 1/5] Add action to label mergeable PRs Developers often have to ping a committer once their PRs are both passing in CI and are approved. This helps facilitate this process by marking such PRs with a label `ready-for-merge` so committers can easily filter for outstanding PRs that need attention. --- .github/workflows/ready_for_merge.yml | 43 ++++++ tests/scripts/git_skip_ci.py | 49 +------ tests/scripts/git_utils.py | 77 +++++++++++ tests/scripts/github_check_pr_is_mergeable.py | 126 ++++++++++++++++++ 4 files changed, 247 insertions(+), 48 deletions(-) create mode 100644 .github/workflows/ready_for_merge.yml create mode 100644 tests/scripts/git_utils.py create mode 100755 tests/scripts/github_check_pr_is_mergeable.py diff --git a/.github/workflows/ready_for_merge.yml b/.github/workflows/ready_for_merge.yml new file mode 100644 index 000000000000..28f745f0ddb1 --- /dev/null +++ b/.github/workflows/ready_for_merge.yml @@ -0,0 +1,43 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Label PRs that have passed CI and are approved + +name: Merge + +on: + status: + pull_request_review: + +concurrency: + group: Merge-${{ github.event.pull_request.number || github.sha }} + cancel-in-progress: true + +jobs: + check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + submodules: "recursive" + - name: Check if PR is ready + env: + SHA: ${{ github.event.pull_request.head.sha || github.event.commit.sha }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -eux + python check_pr_is_ready.py --sha "$SHA" diff --git a/tests/scripts/git_skip_ci.py b/tests/scripts/git_skip_ci.py index 73fcc6490ab8..ac63886c1d91 100755 --- a/tests/scripts/git_skip_ci.py +++ b/tests/scripts/git_skip_ci.py @@ -17,56 +17,9 @@ # under the License. import os -import json import argparse -import subprocess -import re -from urllib import request -from typing import Dict, Tuple, Any - -class GitHubRepo: - def __init__(self, user, repo, token): - self.token = token - self.user = user - self.repo = repo - self.base = f"https://api.github.com/repos/{user}/{repo}/" - - def headers(self): - return { - "Authorization": f"Bearer {self.token}", - } - - def get(self, url: str) -> Dict[str, Any]: - url = self.base + url - print("Requesting", url) - req = request.Request(url, headers=self.headers()) - with request.urlopen(req) as response: - response = json.loads(response.read()) - return response - - -def parse_remote(remote: str) -> Tuple[str, str]: - """ - Get a GitHub (user, repo) pair out of a git remote - """ - if remote.startswith("https://"): - # Parse HTTP remote - parts = remote.split("/") - if len(parts) < 2: - raise RuntimeError(f"Unable to parse remote '{remote}'") - return parts[-2], parts[-1].replace(".git", "") - else: - # Parse SSH remote - m = re.search(r":(.*)/(.*)\.git", remote) - if m is None or len(m.groups()) != 2: - raise RuntimeError(f"Unable to parse remote '{remote}'") - return m.groups() - - -def git(command): - proc = subprocess.run(["git"] + command, stdout=subprocess.PIPE, check=True) - return proc.stdout.decode().strip() +from .git_utils import git, GitHubRepo, parse_remote if __name__ == "__main__": diff --git a/tests/scripts/git_utils.py b/tests/scripts/git_utils.py new file mode 100644 index 000000000000..262f20af4e23 --- /dev/null +++ b/tests/scripts/git_utils.py @@ -0,0 +1,77 @@ +import json +import subprocess +import re +from urllib import request +from typing import Dict, Tuple, Any + + +class GitHubRepo: + def __init__(self, user, repo, token): + self.token = token + self.user = user + self.repo = repo + self.base = f"https://api.github.com/repos/{user}/{repo}/" + + def headers(self): + return { + "Authorization": f"Bearer {self.token}", + } + + def graphql(self, query: str) -> Dict[str, Any]: + return self._post("https://api.github.com/graphql", {"query": query}) + + def _post(self, full_url: str, body: Dict[str, Any]) -> Dict[str, Any]: + print("Requesting", full_url) + req = request.Request(full_url, headers=self.headers(), method="POST") + req.add_header("Content-Type", "application/json; charset=utf-8") + data = json.dumps(body) + data = data.encode("utf-8") + req.add_header("Content-Length", len(data)) + + with request.urlopen(req, data) as response: + response = json.loads(response.read()) + return response + + def post(self, url: str, data: Dict[str, Any]) -> Dict[str, Any]: + return self._post(self.base + url, data) + + def get(self, url: str) -> Dict[str, Any]: + url = self.base + url + print("Requesting", url) + req = request.Request(url, headers=self.headers()) + with request.urlopen(req) as response: + response = json.loads(response.read()) + return response + + def delete(self, url: str) -> Dict[str, Any]: + url = self.base + url + print("Requesting", url) + req = request.Request(url, headers=self.headers(), method="DELETE") + with request.urlopen(req) as response: + response = json.loads(response.read()) + return response + + +def parse_remote(remote: str) -> Tuple[str, str]: + """ + Get a GitHub (user, repo) pair out of a git remote + """ + if remote.startswith("https://"): + # Parse HTTP remote + parts = remote.split("/") + if len(parts) < 2: + raise RuntimeError(f"Unable to parse remote '{remote}'") + return parts[-2], parts[-1].replace(".git", "") + else: + # Parse SSH remote + m = re.search(r":(.*)/(.*)\.git", remote) + if m is None or len(m.groups()) != 2: + raise RuntimeError(f"Unable to parse remote '{remote}'") + return m.groups() + + +def git(command): + command = ["git"] + command + print("Running", command) + proc = subprocess.run(command, stdout=subprocess.PIPE, check=True) + return proc.stdout.decode().strip() diff --git a/tests/scripts/github_check_pr_is_mergeable.py b/tests/scripts/github_check_pr_is_mergeable.py new file mode 100755 index 000000000000..03859416ecd7 --- /dev/null +++ b/tests/scripts/github_check_pr_is_mergeable.py @@ -0,0 +1,126 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import os +import json +import argparse +from urllib import error +from typing import Dict, Tuple, Any + +from .git_utils import git, GitHubRepo, parse_remote + + +def commit_query(repo: str, user: str, sha: str) -> str: + """ + Build the GraphQL query to find a PR linked from a commit along with its + latest build status + """ + return f""" + {{ + repository(name: "{repo}", owner: "{user}") {{ + object(oid: "{sha}") {{ + ... on Commit {{ + associatedPullRequests(last:1) {{ + nodes {{ + number + reviewDecision + commits(last:1) {{ + nodes {{ + commit {{ + statusCheckRollup {{ + contexts(last:100) {{ + nodes {{ + ... on CheckRun {{ + conclusion + status + name + checkSuite {{ + workflowRun {{ + workflow {{ + name + }} + }} + }} + }} + ... on StatusContext {{ + context + state + }} + }} + }} + }} + }} + }} + }} + }} + }} + }} + }} + }} + }}""" + + +def is_pr_ready(data: Any) -> bool: + """ + Returns true if a PR is approved and all of its statuses are SUCCESS + """ + approved = data["reviewDecision"] == "APPROVED" + print("Is approved?", approved) + + statuses = data["commits"]["nodes"][0]["commit"]["statusCheckRollup"]["contexts"]["nodes"] + unified_statuses = [] + for status in statuses: + if "context" in status: + # Parse non-GHA status + unified_statuses.append((status["context"], status["state"] == "SUCCESS")) + else: + # Parse GitHub Actions item + workflow = status["checkSuite"]["workflowRun"]["workflow"]["name"] + name = f"{workflow} / {status['name']}" + unified_statuses.append((name, status["conclusion"] == "SUCCESS")) + + print("Got statuses:", json.dumps(unified_statuses, indent=2)) + passed_ci = all(status for name, status in unified_statuses) + return approved and passed_ci + + +if __name__ == "__main__": + help = "Adds label to PRs that have passed CI and are approved" + parser = argparse.ArgumentParser(description=help) + parser.add_argument("--sha", required=True) + parser.add_argument("--remote", default="origin", help="ssh remote to parse") + parser.add_argument("--label", default="ready-for-merge", help="label to add") + args = parser.parse_args() + + remote = git(["config", "--get", f"remote.{args.remote}.url"]) + user, repo = parse_remote(remote) + github = GitHubRepo(token=os.environ["GITHUB_TOKEN"], user=user, repo=repo) + + data = github.graphql(commit_query(repo, user, args.sha)) + pr = data["data"]["repository"]["object"]["associatedPullRequests"]["nodes"][0] + + if is_pr_ready(pr): + print("PR passed CI and is approved, labelling...") + github.post(f"issues/{pr['number']}/labels", {"labels": [args.label]}) + else: + print("PR is not ready for merge") + try: + github.delete(f"issues/{pr['number']}/labels/{args.label}") + except error.HTTPError as e: + print(e) + print("Failed to remove label (it may not have been there at all)") From f87c1a00a872794784bc6546acff9de9441250e8 Mon Sep 17 00:00:00 2001 From: driazati Date: Fri, 14 Jan 2022 13:44:19 -0800 Subject: [PATCH 2/5] Fix lint and add tests --- .github/workflows/ready_for_merge.yml | 2 +- tests/python/unittest/test_ci.py | 76 +++++++++++++++++++ tests/scripts/git_skip_ci.py | 2 +- tests/scripts/git_utils.py | 18 +++++ tests/scripts/github_check_pr_is_mergeable.py | 36 ++++++--- 5 files changed, 121 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ready_for_merge.yml b/.github/workflows/ready_for_merge.yml index 28f745f0ddb1..4e574ea39773 100644 --- a/.github/workflows/ready_for_merge.yml +++ b/.github/workflows/ready_for_merge.yml @@ -40,4 +40,4 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | set -eux - python check_pr_is_ready.py --sha "$SHA" + python tests/scripts/github_check_pr_is_mergeable.py --sha "$SHA" diff --git a/tests/python/unittest/test_ci.py b/tests/python/unittest/test_ci.py index ac7e6cdd7c29..d3b777cb082b 100644 --- a/tests/python/unittest/test_ci.py +++ b/tests/python/unittest/test_ci.py @@ -18,6 +18,7 @@ import pathlib import subprocess import sys +import json import tempfile import pytest @@ -25,6 +26,81 @@ REPO_ROOT = pathlib.Path(__file__).resolve().parent.parent.parent.parent +def test_pr_is_mergable(): + is_mergable_script = REPO_ROOT / "tests" / "scripts" / "github_check_pr_is_mergeable.py" + + def run(decision, statuses, mergeable): + # Mock out the response from GitHub's API + data = { + "reviewDecision": decision, + "commits": { + "nodes": [{"commit": {"statusCheckRollup": {"contexts": {"nodes": statuses}}}}] + }, + } + proc = subprocess.run( + [str(is_mergable_script), "--pr-json", json.dumps(data)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + encoding="utf-8", + ) + if proc.returncode != 0: + raise RuntimeError(f"Process failed:\nstdout:\n{proc.stdout}\n\nstderr:\n{proc.stderr}") + + # Find the relevant string in the output + if mergeable: + assert "PR passed CI and is approved, labelling" in proc.stdout + else: + assert "PR is not ready for merge" in proc.stdout + + # mergeable should be true iff all statuses are successful and PR is approved + run(decision="CHANGES_REQUESTED", statuses=[], mergeable=False) + run(decision="APPROVED", statuses=[], mergeable=True) + run( + decision="CHANGES_REQUESTED", + statuses=[ + { + "context": "abc", + "state": "FAILED", + } + ], + mergeable=False, + ) + run( + decision="APPROVED", + statuses=[ + { + "context": "abc", + "state": "FAILED", + } + ], + mergeable=False, + ) + run( + decision="APPROVED", + statuses=[ + { + "context": "abc", + "state": "SUCCESS", + } + ], + mergeable=True, + ) + run( + decision="APPROVED", + statuses=[ + { + "context": "abc", + "state": "SUCCESS", + }, + { + "context": "abc2", + "state": "FAILURE", + }, + ], + mergeable=False, + ) + + def test_skip_ci(): skip_ci_script = REPO_ROOT / "tests" / "scripts" / "git_skip_ci.py" diff --git a/tests/scripts/git_skip_ci.py b/tests/scripts/git_skip_ci.py index ac63886c1d91..c4b88676c34f 100755 --- a/tests/scripts/git_skip_ci.py +++ b/tests/scripts/git_skip_ci.py @@ -19,7 +19,7 @@ import os import argparse -from .git_utils import git, GitHubRepo, parse_remote +from git_utils import git, GitHubRepo, parse_remote if __name__ == "__main__": diff --git a/tests/scripts/git_utils.py b/tests/scripts/git_utils.py index 262f20af4e23..f2927f1e3ab7 100644 --- a/tests/scripts/git_utils.py +++ b/tests/scripts/git_utils.py @@ -1,3 +1,21 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + import json import subprocess import re diff --git a/tests/scripts/github_check_pr_is_mergeable.py b/tests/scripts/github_check_pr_is_mergeable.py index 03859416ecd7..7a286edbe23e 100755 --- a/tests/scripts/github_check_pr_is_mergeable.py +++ b/tests/scripts/github_check_pr_is_mergeable.py @@ -22,7 +22,7 @@ from urllib import error from typing import Dict, Tuple, Any -from .git_utils import git, GitHubRepo, parse_remote +from git_utils import git, GitHubRepo, parse_remote def commit_query(repo: str, user: str, sha: str) -> str: @@ -102,25 +102,39 @@ def is_pr_ready(data: Any) -> bool: if __name__ == "__main__": help = "Adds label to PRs that have passed CI and are approved" parser = argparse.ArgumentParser(description=help) - parser.add_argument("--sha", required=True) + parser.add_argument("--sha") parser.add_argument("--remote", default="origin", help="ssh remote to parse") parser.add_argument("--label", default="ready-for-merge", help="label to add") + parser.add_argument( + "--pr-json", help="(testing) PR data to use instead of fetching from GitHub" + ) args = parser.parse_args() remote = git(["config", "--get", f"remote.{args.remote}.url"]) user, repo = parse_remote(remote) - github = GitHubRepo(token=os.environ["GITHUB_TOKEN"], user=user, repo=repo) - data = github.graphql(commit_query(repo, user, args.sha)) - pr = data["data"]["repository"]["object"]["associatedPullRequests"]["nodes"][0] + is_testing = args.pr_json is not None + if not is_testing and args.sha is None: + print("--sha must be used outside of testing") + exit(1) + + if args.pr_json: + pr = json.loads(args.pr_json) + else: + github = GitHubRepo(token=os.environ["GITHUB_TOKEN"], user=user, repo=repo) + + data = github.graphql(commit_query(repo, user, args.sha)) + pr = data["data"]["repository"]["object"]["associatedPullRequests"]["nodes"][0] if is_pr_ready(pr): print("PR passed CI and is approved, labelling...") - github.post(f"issues/{pr['number']}/labels", {"labels": [args.label]}) + if not is_testing: + github.post(f"issues/{pr['number']}/labels", {"labels": [args.label]}) else: print("PR is not ready for merge") - try: - github.delete(f"issues/{pr['number']}/labels/{args.label}") - except error.HTTPError as e: - print(e) - print("Failed to remove label (it may not have been there at all)") + if not is_testing: + try: + github.delete(f"issues/{pr['number']}/labels/{args.label}") + except error.HTTPError as e: + print(e) + print("Failed to remove label (it may not have been there at all)") From 96dcee9625a42c80a638e234d205aae01996eeee Mon Sep 17 00:00:00 2001 From: driazati Date: Fri, 14 Jan 2022 13:59:33 -0800 Subject: [PATCH 3/5] Add Action to add cc'ed people as reviewers This provides a mechanism for non-triager/reviewer/committer PR authors to request reviews through GitHub. Anyone that is referenced by `cc @username` in a PR body will be added as a reviewer (GitHub will limit the reviewers to those with actual permissions to leave reviews so the script to add can be simple). --- .github/workflows/cc_bot.yml | 45 +++++++++++++++++ tests/python/unittest/test_ci.py | 27 ++++++++++ tests/scripts/github_cc_reviewers.py | 73 ++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+) create mode 100644 .github/workflows/cc_bot.yml create mode 100755 tests/scripts/github_cc_reviewers.py diff --git a/.github/workflows/cc_bot.yml b/.github/workflows/cc_bot.yml new file mode 100644 index 000000000000..1aae748c200d --- /dev/null +++ b/.github/workflows/cc_bot.yml @@ -0,0 +1,45 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# GH actions. +# We use it to cover windows and mac builds +# Jenkins is still the primary CI + +name: PR + +on: + pull_request_target: + types: [assigned, opened, synchronize, reopened] + +concurrency: + group: PR-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + cc-reviewers: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + submodules: "recursive" + - name: Check if PR is ready + env: + PR: ${{ toJson(github.event.pull_request) }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -eux + python tests/scripts/github_cc_reviewers.py diff --git a/tests/python/unittest/test_ci.py b/tests/python/unittest/test_ci.py index d3b777cb082b..21c195d5f585 100644 --- a/tests/python/unittest/test_ci.py +++ b/tests/python/unittest/test_ci.py @@ -26,6 +26,33 @@ REPO_ROOT = pathlib.Path(__file__).resolve().parent.parent.parent.parent +def test_cc_reviewers(): + reviewers_script = REPO_ROOT / "tests" / "scripts" / "github_cc_reviewers.py" + + def run(pr_body, expected_reviewers): + proc = subprocess.run( + [str(reviewers_script), "--dry-run"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env={"PR": json.dumps({"number": 1, "body": pr_body})}, + encoding="utf-8", + ) + if proc.returncode != 0: + raise RuntimeError(f"Process failed:\nstdout:\n{proc.stdout}\n\nstderr:\n{proc.stderr}") + + assert proc.stdout.strip().endswith(f"Adding reviewers: {expected_reviewers}") + + run(pr_body="abc", expected_reviewers=[]) + run(pr_body="cc @abc", expected_reviewers=["abc"]) + run(pr_body="cc @", expected_reviewers=[]) + run(pr_body="cc @abc @def", expected_reviewers=["abc", "def"]) + run(pr_body="some text cc @abc @def something else", expected_reviewers=["abc", "def"]) + run( + pr_body="some text cc @abc @def something else\n\n another cc @zzz z", + expected_reviewers=["abc", "def", "zzz"], + ) + + def test_pr_is_mergable(): is_mergable_script = REPO_ROOT / "tests" / "scripts" / "github_check_pr_is_mergeable.py" diff --git a/tests/scripts/github_cc_reviewers.py b/tests/scripts/github_cc_reviewers.py new file mode 100755 index 000000000000..48420822ad55 --- /dev/null +++ b/tests/scripts/github_cc_reviewers.py @@ -0,0 +1,73 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import os +import json +import argparse +import re +from typing import Dict, Any, List + + +from git_utils import git, GitHubRepo, parse_remote + + +def find_reviewers(body: str) -> List[str]: + print(f"Parsing body:\n{body}") + matches = re.findall(r"(cc( @[-A-Za-z0-9]+)+)", body, flags=re.MULTILINE) + matches = [full for full, last in matches] + + print("Found matches:", matches) + reviewers = [] + for match in matches: + if match.startswith("cc "): + match = match.replace("cc ", "") + users = [x.strip() for x in match.split("@")] + reviewers += users + + reviewers = set(x for x in reviewers if x != "") + return sorted(list(reviewers)) + + +if __name__ == "__main__": + help = "Add @cc'ed people in a PR body as reviewers" + parser = argparse.ArgumentParser(description=help) + parser.add_argument("--remote", default="origin", help="ssh remote to parse") + parser.add_argument( + "--dry-run", + action="store_true", + default=False, + help="run but don't send any request to GitHub", + ) + args = parser.parse_args() + + remote = git(["config", "--get", f"remote.{args.remote}.url"]) + user, repo = parse_remote(remote) + + pr = json.loads(os.environ["PR"]) + + number = pr["number"] + body = pr["body"] + if body is None: + body = "" + + to_add = find_reviewers(body) + print("Adding reviewers:", to_add) + + if not args.dry_run: + github = GitHubRepo(token=os.environ["GITHUB_TOKEN"], user=user, repo=repo) + github.post(f"pulls/{number}/requested_reviewers", {"reviewers": to_add}) From 8b4e74c1440d009540d00fc0b8a5b3343f9491b3 Mon Sep 17 00:00:00 2001 From: driazati Date: Tue, 18 Jan 2022 16:44:23 -0800 Subject: [PATCH 4/5] remove merge bot stuff --- .github/workflows/ready_for_merge.yml | 43 ------ tests/python/unittest/test_ci.py | 75 ---------- tests/scripts/github_check_pr_is_mergeable.py | 140 ------------------ 3 files changed, 258 deletions(-) delete mode 100644 .github/workflows/ready_for_merge.yml delete mode 100755 tests/scripts/github_check_pr_is_mergeable.py diff --git a/.github/workflows/ready_for_merge.yml b/.github/workflows/ready_for_merge.yml deleted file mode 100644 index 4e574ea39773..000000000000 --- a/.github/workflows/ready_for_merge.yml +++ /dev/null @@ -1,43 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -# Label PRs that have passed CI and are approved - -name: Merge - -on: - status: - pull_request_review: - -concurrency: - group: Merge-${{ github.event.pull_request.number || github.sha }} - cancel-in-progress: true - -jobs: - check: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - submodules: "recursive" - - name: Check if PR is ready - env: - SHA: ${{ github.event.pull_request.head.sha || github.event.commit.sha }} - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - set -eux - python tests/scripts/github_check_pr_is_mergeable.py --sha "$SHA" diff --git a/tests/python/unittest/test_ci.py b/tests/python/unittest/test_ci.py index 21c195d5f585..0c80617985ee 100644 --- a/tests/python/unittest/test_ci.py +++ b/tests/python/unittest/test_ci.py @@ -53,81 +53,6 @@ def run(pr_body, expected_reviewers): ) -def test_pr_is_mergable(): - is_mergable_script = REPO_ROOT / "tests" / "scripts" / "github_check_pr_is_mergeable.py" - - def run(decision, statuses, mergeable): - # Mock out the response from GitHub's API - data = { - "reviewDecision": decision, - "commits": { - "nodes": [{"commit": {"statusCheckRollup": {"contexts": {"nodes": statuses}}}}] - }, - } - proc = subprocess.run( - [str(is_mergable_script), "--pr-json", json.dumps(data)], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - encoding="utf-8", - ) - if proc.returncode != 0: - raise RuntimeError(f"Process failed:\nstdout:\n{proc.stdout}\n\nstderr:\n{proc.stderr}") - - # Find the relevant string in the output - if mergeable: - assert "PR passed CI and is approved, labelling" in proc.stdout - else: - assert "PR is not ready for merge" in proc.stdout - - # mergeable should be true iff all statuses are successful and PR is approved - run(decision="CHANGES_REQUESTED", statuses=[], mergeable=False) - run(decision="APPROVED", statuses=[], mergeable=True) - run( - decision="CHANGES_REQUESTED", - statuses=[ - { - "context": "abc", - "state": "FAILED", - } - ], - mergeable=False, - ) - run( - decision="APPROVED", - statuses=[ - { - "context": "abc", - "state": "FAILED", - } - ], - mergeable=False, - ) - run( - decision="APPROVED", - statuses=[ - { - "context": "abc", - "state": "SUCCESS", - } - ], - mergeable=True, - ) - run( - decision="APPROVED", - statuses=[ - { - "context": "abc", - "state": "SUCCESS", - }, - { - "context": "abc2", - "state": "FAILURE", - }, - ], - mergeable=False, - ) - - def test_skip_ci(): skip_ci_script = REPO_ROOT / "tests" / "scripts" / "git_skip_ci.py" diff --git a/tests/scripts/github_check_pr_is_mergeable.py b/tests/scripts/github_check_pr_is_mergeable.py deleted file mode 100755 index 7a286edbe23e..000000000000 --- a/tests/scripts/github_check_pr_is_mergeable.py +++ /dev/null @@ -1,140 +0,0 @@ -#!/usr/bin/env python3 -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -import os -import json -import argparse -from urllib import error -from typing import Dict, Tuple, Any - -from git_utils import git, GitHubRepo, parse_remote - - -def commit_query(repo: str, user: str, sha: str) -> str: - """ - Build the GraphQL query to find a PR linked from a commit along with its - latest build status - """ - return f""" - {{ - repository(name: "{repo}", owner: "{user}") {{ - object(oid: "{sha}") {{ - ... on Commit {{ - associatedPullRequests(last:1) {{ - nodes {{ - number - reviewDecision - commits(last:1) {{ - nodes {{ - commit {{ - statusCheckRollup {{ - contexts(last:100) {{ - nodes {{ - ... on CheckRun {{ - conclusion - status - name - checkSuite {{ - workflowRun {{ - workflow {{ - name - }} - }} - }} - }} - ... on StatusContext {{ - context - state - }} - }} - }} - }} - }} - }} - }} - }} - }} - }} - }} - }} - }}""" - - -def is_pr_ready(data: Any) -> bool: - """ - Returns true if a PR is approved and all of its statuses are SUCCESS - """ - approved = data["reviewDecision"] == "APPROVED" - print("Is approved?", approved) - - statuses = data["commits"]["nodes"][0]["commit"]["statusCheckRollup"]["contexts"]["nodes"] - unified_statuses = [] - for status in statuses: - if "context" in status: - # Parse non-GHA status - unified_statuses.append((status["context"], status["state"] == "SUCCESS")) - else: - # Parse GitHub Actions item - workflow = status["checkSuite"]["workflowRun"]["workflow"]["name"] - name = f"{workflow} / {status['name']}" - unified_statuses.append((name, status["conclusion"] == "SUCCESS")) - - print("Got statuses:", json.dumps(unified_statuses, indent=2)) - passed_ci = all(status for name, status in unified_statuses) - return approved and passed_ci - - -if __name__ == "__main__": - help = "Adds label to PRs that have passed CI and are approved" - parser = argparse.ArgumentParser(description=help) - parser.add_argument("--sha") - parser.add_argument("--remote", default="origin", help="ssh remote to parse") - parser.add_argument("--label", default="ready-for-merge", help="label to add") - parser.add_argument( - "--pr-json", help="(testing) PR data to use instead of fetching from GitHub" - ) - args = parser.parse_args() - - remote = git(["config", "--get", f"remote.{args.remote}.url"]) - user, repo = parse_remote(remote) - - is_testing = args.pr_json is not None - if not is_testing and args.sha is None: - print("--sha must be used outside of testing") - exit(1) - - if args.pr_json: - pr = json.loads(args.pr_json) - else: - github = GitHubRepo(token=os.environ["GITHUB_TOKEN"], user=user, repo=repo) - - data = github.graphql(commit_query(repo, user, args.sha)) - pr = data["data"]["repository"]["object"]["associatedPullRequests"]["nodes"][0] - - if is_pr_ready(pr): - print("PR passed CI and is approved, labelling...") - if not is_testing: - github.post(f"issues/{pr['number']}/labels", {"labels": [args.label]}) - else: - print("PR is not ready for merge") - if not is_testing: - try: - github.delete(f"issues/{pr['number']}/labels/{args.label}") - except error.HTTPError as e: - print(e) - print("Failed to remove label (it may not have been there at all)") From c35db25690d0a7f042ef2ad5019772317764a2f6 Mon Sep 17 00:00:00 2001 From: driazati Date: Tue, 18 Jan 2022 17:07:45 -0800 Subject: [PATCH 5/5] Fix target triggers --- .github/workflows/cc_bot.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cc_bot.yml b/.github/workflows/cc_bot.yml index 1aae748c200d..dd50eba79358 100644 --- a/.github/workflows/cc_bot.yml +++ b/.github/workflows/cc_bot.yml @@ -22,8 +22,9 @@ name: PR on: + # See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target pull_request_target: - types: [assigned, opened, synchronize, reopened] + types: [assigned, opened, synchronize, reopened, edited, ready_for_review] concurrency: group: PR-${{ github.event.pull_request.number }} @@ -36,7 +37,7 @@ jobs: - uses: actions/checkout@v2 with: submodules: "recursive" - - name: Check if PR is ready + - name: Add cc'ed reviewers env: PR: ${{ toJson(github.event.pull_request) }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}