Skip to content

Commit

Permalink
Merge pull request #758 from conda-forge/admin-token
Browse files Browse the repository at this point in the history
feat: use admin token for merge and comments when doing automerge
  • Loading branch information
beckermr authored Nov 2, 2024
2 parents fb740ba + 1ff5271 commit b31b437
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 23 deletions.
5 changes: 3 additions & 2 deletions .github/workflows/automerge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,14 @@ jobs:
- name: automerge
run: |
git config --global user.email "79913779+conda-forge-curator[bot]@users.noreply.github.com"
git config --global user.name "conda-forge-curator[bot]"
git config --global user.name "conda-forge-webservices[bot]"
git config --global user.email "91080706+conda-forge-webservices[bot]@users.noreply.github.com"
git config --global pull.rebase false
conda-forge-webservices-automerge \
--repo=${{ inputs.repo }} \
--sha=${{ inputs.sha }}
env:
GH_TOKEN: ${{ steps.generate_token.outputs.token }}
GH_TOKEN_FOR_ADMIN: ${{ secrets.CF_ADMIN_GITHUB_TOKEN }}

Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
get_git_patch_relative_to_commit,
mark_pr_as_ready_for_review,
)
from .api_sessions import create_api_sessions
from .api_sessions import create_api_sessions, create_api_sessions_for_admin
from .rerendering import rerender
from .linting import (
make_lint_comment,
Expand Down Expand Up @@ -488,7 +488,10 @@ def main_automerge(repo, sha):
gh_repo = gh.get_repo(full_repo_name)
for pr in gh_repo.get_pulls():
if pr.head.sha == sha:
automerge_pr(gh_repo, pr)
_, gh_for_admin = create_api_sessions_for_admin()
gh_repo_for_admin = gh_for_admin.get_repo(full_repo_name)
pr_for_admin = gh_repo_for_admin.get_pull(pr.number)
automerge_pr(gh_repo, pr, pr_for_admin)
found_pr = True

if not found_pr:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,20 @@ def create_api_sessions():
return _create_api_sessions(os.environ["GH_TOKEN"])


@lru_cache(maxsize=1)
def create_api_sessions_for_admin():
"""Create API sessions for GitHub using the admin token.
Returns
-------
session : requests.Session
A `requests` session w/ the beta `check_run` API configured.
gh : github.MainClass.Github
A `Github` object from the PyGithub package.
"""
return _create_api_sessions(os.environ["GH_TOKEN_FOR_ADMIN"])


@lru_cache(maxsize=2)
def _create_api_sessions(github_token):
# based on
# https://alexwlchan.net/2019/03/
Expand Down
40 changes: 27 additions & 13 deletions conda_forge_webservices/github_actions_integration/automerge.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,9 @@ def _no_extra_pr_commits(pr):
return all(e.event != "committed" for e in events[label_ind + 1 :])


def _check_pr(pr: PullRequest, cfg) -> tuple[bool, str | None]:
def _check_pr(
pr: PullRequest, pr_for_admin: PullRequest, cfg
) -> tuple[bool, str | None]:
"""make sure a PR is ok to automerge"""

pr_has_automerge_label = any(label.name == "automerge" for label in pr.get_labels())
Expand All @@ -351,7 +353,7 @@ def _check_pr(pr: PullRequest, cfg) -> tuple[bool, str | None]:
return True, None
else:
pr.remove_from_labels("automerge")
pr.create_issue_comment(
pr_for_admin.create_issue_comment(
"""\
Hi! This is the friendly conda-forge automerge bot!
Expand All @@ -378,7 +380,7 @@ def _check_pr(pr: PullRequest, cfg) -> tuple[bool, str | None]:
committers = {getattr(c.author, "login", None) for c in pr.get_commits()}
if not all(c in ALLOWED_USERS for c in committers):
_comment_on_pr_with_race(
pr,
pr_for_admin,
"""\
Hi! This is the friendly conda-forge automerge bot!
Expand Down Expand Up @@ -432,9 +434,13 @@ def _comment_on_pr(pr, stats, msg):
_comment_on_pr_with_race(pr, comment, check_slug)


def _automerge_pr(repo: Repository, pr: PullRequest) -> tuple[bool, str | None]:
def _automerge_pr(
repo: Repository,
pr: PullRequest,
pr_for_admin: PullRequest,
) -> tuple[bool, str | None]:
cfg = _get_conda_forge_config(pr)
allowed, msg = _check_pr(pr, cfg)
allowed, msg = _check_pr(pr, pr_for_admin, cfg)

if not allowed:
return False, msg
Expand All @@ -452,7 +458,7 @@ def _automerge_pr(repo: Repository, pr: PullRequest) -> tuple[bool, str | None]:
status_states, check_states, req_checks_and_states
)
if not ok:
_comment_on_pr(pr, final_statuses, "not passing and not merged.")
_comment_on_pr(pr_for_admin, final_statuses, "not passing and not merged.")
return False, "PR has failing or pending statuses/checks"

# make sure PR is mergeable and not already merged
Expand All @@ -464,15 +470,15 @@ def _automerge_pr(repo: Repository, pr: PullRequest) -> tuple[bool, str | None]:

if pr.mergeable is None or not pr.mergeable:
_comment_on_pr(
pr,
pr_for_admin,
final_statuses,
f"passing, but not in a mergeable state (mergeable={pr.mergeable}).",
)
return False, f"PR merge issue: mergeable={pr.mergeable}"

# we're good - now merge
try:
merge_status = pr.merge(
merge_status = pr_for_admin.merge(
commit_message="automerged PR by conda-forge/automerge-action",
commit_title=f"{pr.title} (#{pr.number})",
merge_method="merge",
Expand All @@ -497,26 +503,34 @@ def _automerge_pr(repo: Repository, pr: PullRequest) -> tuple[bool, str | None]:

if not merge_status_merged:
_comment_on_pr(
pr,
pr_for_admin,
final_statuses,
f"passing, but could not be merged (error={merge_status_message}).",
)
return (False, f"PR could not be merged: {merge_status_message}")
else:
# use a smaller check_race here to make sure this one is prompt
_comment_on_pr(pr, final_statuses, "passing and merged! Have a great day!")
_comment_on_pr(
pr_for_admin, final_statuses, "passing and merged! Have a great day!"
)
return True, "all is well :)"


def automerge_pr(repo: Repository, pr: PullRequest) -> tuple[bool, str | None]:
def automerge_pr(
repo: Repository, pr: PullRequest, pr_for_admin: PullRequest
) -> tuple[bool, str | None]:
"""Possibly automerge a PR.
Parameters
----------
repo : github.Repository.Repository
A `Repository` object for the given repo from the PyGithub package.
pr : github.PullRequest.PullRequest
A `PullRequest` object for the given PR from the PyGithub package.
A `PullRequest` object for the given PR from the PyGithub package
that is used for API-request heavy operations.
pr_for_admin : github.PullRequest.PullRequest
A `PullRequest` object for the given PR from the PyGithub package
that is used only to comment on and/or merge the PR.
Returns
-------
Expand All @@ -525,7 +539,7 @@ def automerge_pr(repo: Repository, pr: PullRequest) -> tuple[bool, str | None]:
reason : str
The reason the merge worked or did not work.
"""
did_merge, reason = _automerge_pr(repo, pr)
did_merge, reason = _automerge_pr(repo, pr, pr_for_admin)

if did_merge:
LOGGER.info("MERGED PR %s on %s: %s", pr.number, repo.full_name, reason)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,17 @@ def test_automerge_pr_bad_user(get_cfg_mock):
pr = MagicMock()
pr.user.login = "blah"

did_merge, reason = automerge_pr(repo, pr)
pr_for_admin = MagicMock()
pr_for_admin.user.login = "regro-cf-autotick-bot"

did_merge, reason = automerge_pr(repo, pr, pr_for_admin)

assert not did_merge
assert "user blah" in reason
get_cfg_mock.assert_called_once_with(pr)
pr_for_admin.create_issue_comment.assert_not_called()
pr_for_admin.get_issue_comments.assert_not_called()
pr_for_admin.merge.assert_not_called()


@unittest.mock.patch(
Expand All @@ -36,11 +42,17 @@ def test_automerge_pr_no_title_slug(get_cfg_mock):
pr.user.login = "regro-cf-autotick-bot"
pr.title = "blah"

did_merge, reason = automerge_pr(repo, pr)
pr_for_admin = MagicMock()
pr_for_admin.user.login = "regro-cf-autotick-bot"

did_merge, reason = automerge_pr(repo, pr, pr_for_admin)

assert not did_merge
assert "slug in the title" in reason
get_cfg_mock.assert_called_once_with(pr)
pr_for_admin.create_issue_comment.assert_not_called()
pr_for_admin.get_issue_comments.assert_not_called()
pr_for_admin.merge.assert_not_called()


@pytest.mark.parametrize(
Expand All @@ -63,11 +75,17 @@ def test_automerge_pr_feedstock_off(get_cfg_mock, cfg):
pr.user.login = "regro-cf-autotick-bot"
pr.title = "[bot-automerge] blah"

did_merge, reason = automerge_pr(repo, pr)
pr_for_admin = MagicMock()
pr_for_admin.user.login = "regro-cf-autotick-bot"

did_merge, reason = automerge_pr(repo, pr, pr_for_admin)

assert not did_merge
assert "off for this feedstock" in reason
get_cfg_mock.assert_called_once_with(pr)
pr_for_admin.create_issue_comment.assert_not_called()
pr_for_admin.get_issue_comments.assert_not_called()
pr_for_admin.merge.assert_not_called()


@pytest.mark.parametrize("fail", ["check", "status"])
Expand Down Expand Up @@ -103,14 +121,21 @@ def test_automerge_pr_feedstock_status_or_check_fail(
pr.user.login = "regro-cf-autotick-bot"
pr.title = "[bot-automerge] blah"

did_merge, reason = automerge_pr(repo, pr)
pr_for_admin = MagicMock()
pr_for_admin.user.login = "regro-cf-autotick-bot"
pr_for_admin.get_issue_comments.return_value = []

did_merge, reason = automerge_pr(repo, pr, pr_for_admin)

assert not did_merge
assert "pending statuses" in reason
get_cfg_mock.assert_called_once_with(pr)
check_mock.assert_called_once_with(repo, pr)
stat_mock.assert_called_once_with(repo, pr)
req_mock.assert_called_once_with(pr, get_cfg_mock.return_value)
pr_for_admin.create_issue_comment.assert_called_once()
pr_for_admin.get_issue_comments.assert_called()
pr_for_admin.merge.assert_not_called()


@unittest.mock.patch(
Expand Down Expand Up @@ -140,11 +165,17 @@ def test_automerge_pr_feedstock_no_statuses_or_checks(
pr.user.login = "regro-cf-autotick-bot"
pr.title = "[bot-automerge] blah"

did_merge, reason = automerge_pr(repo, pr)
pr_for_admin = MagicMock()
pr_for_admin.user.login = "regro-cf-autotick-bot"

did_merge, reason = automerge_pr(repo, pr, pr_for_admin)

assert not did_merge
assert "At least one status or check must be required" in reason
get_cfg_mock.assert_called_once_with(pr)
check_mock.assert_called_once_with(repo, pr)
stat_mock.assert_called_once_with(repo, pr)
req_mock.assert_called_once_with(pr, get_cfg_mock.return_value)
pr_for_admin.create_issue_comment.assert_not_called()
pr_for_admin.get_issue_comments.assert_not_called()
pr_for_admin.merge.assert_not_called()

0 comments on commit b31b437

Please sign in to comment.