Skip to content

Commit d193bb5

Browse files
committed
Migrate from github3.py to PyGithub
1 parent ff9f99a commit d193bb5

10 files changed

Lines changed: 310 additions & 155 deletions

File tree

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ readme = "README.md"
1010
requires-python = ">=3.11"
1111

1212
dependencies = [
13-
"github3-py==4.0.1",
13+
"PyGithub==2.9.1",
1414
"gitpython==3.1.46",
1515
"requests==2.33.1",
1616
]

rebasebot/bot.py

Lines changed: 43 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,11 @@
2525

2626
import git
2727
import git.compat
28-
import github3
2928
import requests
3029
from git.objects import Commit
31-
from github3.pulls import ShortPullRequest
32-
from github3.repos.commit import ShortCommit
33-
from github3.repos.repo import Repository
30+
from github import GithubException
31+
from github.PullRequest import PullRequest
32+
from github.Repository import Repository
3433

3534
from rebasebot import lifecycle_hooks
3635
from rebasebot.github import GithubAppProvider, GitHubBranch
@@ -82,9 +81,9 @@ def _needs_rebase(gitwd: git.Repo, source: GitHubBranch, dest: GitHubBranch) ->
8281

8382
def _is_pr_merged(pr_number: int, source_repo: Repository, gitwd: git.Repo, source_branch: str) -> bool:
8483
logging.info("Checking that PR %s has been merged and is included in %s", pr_number, source_branch)
85-
gh_pr = source_repo.pull_request(pr_number)
84+
gh_pr = source_repo.get_pull(pr_number)
8685

87-
if not gh_pr.is_merged():
86+
if not gh_pr.merged:
8887
return False
8988

9089
merge_commit_sha = gh_pr.merge_commit_sha
@@ -633,11 +632,10 @@ def _cherrypick_art_pull_request(
633632
that updates the build image, it includes it in the rebase.
634633
"""
635634
logging.info("Checking for ART pull request")
636-
for pull_request in dest_repo.pull_requests(state="open", base=f"{dest.branch}"):
637-
assert isinstance(pull_request, ShortPullRequest) # type hint
635+
for pull_request in dest_repo.get_pulls(state="open", base=dest.branch):
638636
if "consistent with ART" in pull_request.title and pull_request.user.login == "openshift-bot":
639637
logging.info(f"Found open ART image update pull requst: {pull_request.title}")
640-
remote = pull_request.head.repository
638+
remote = pull_request.head.repo
641639
remote_name = remote.name
642640
if remote_name in gitwd.remotes:
643641
gitwd.remotes[remote_name].set_url(remote.html_url)
@@ -646,8 +644,7 @@ def _cherrypick_art_pull_request(
646644

647645
gitwd.remotes[remote_name].fetch(pull_request.head.ref)
648646

649-
for commit in pull_request.commits():
650-
assert isinstance(commit, ShortCommit)
647+
for commit in pull_request.get_commits():
651648
_safe_cherry_pick(
652649
gitwd=gitwd,
653650
sha=commit.sha,
@@ -684,14 +681,19 @@ def _is_pr_required(gitwd: git.Repo, rebase: GitHubBranch, dest: GitHubBranch) -
684681
return True
685682

686683

687-
def _is_pr_available(dest_repo: Repository, dest: GitHubBranch, rebase: GitHubBranch) -> tuple[ShortPullRequest, bool]:
684+
def _is_pr_available(
685+
dest_repo: Repository, dest: GitHubBranch, rebase: GitHubBranch
686+
) -> tuple[PullRequest | None, bool]:
688687
logging.info("Checking for existing pull request")
689688

690-
pull_requests = dest_repo.pull_requests(base=dest.branch, state="open")
689+
pull_requests = dest_repo.get_pulls(base=dest.branch, state="open")
691690
# Github does not support filtering cross-repository pull requests if both repositories
692691
# are owned by the same organization. We must filter client side.
693692
for pr in pull_requests:
694-
pr_repo = pr.as_dict()["head"]["repo"]["full_name"]
693+
if pr.head.repo is None:
694+
logging.warning(f"Skipping PR with deleted head repository: {pr.html_url}")
695+
continue
696+
pr_repo = pr.head.repo.full_name
695697
if pr_repo == f"{rebase.ns}/{rebase.name}" and pr.head.ref == rebase.branch:
696698
logging.info('Found existing pull request: "%s" %s', pr.title, pr.html_url)
697699
return pr, True
@@ -702,7 +704,7 @@ def _is_pr_available(dest_repo: Repository, dest: GitHubBranch, rebase: GitHubBr
702704

703705
def _create_pr(
704706
*,
705-
gh_app: github3.GitHub,
707+
gh_app,
706708
dest: GitHubBranch,
707709
source: GitHubBranch,
708710
rebase: GitHubBranch,
@@ -717,30 +719,19 @@ def _create_pr(
717719

718720
logging.info("Creating a pull request")
719721

720-
# FIXME(rmanak): This hack is because github3 doesn't support setting
721-
# head_repo param when creating a PR.
722-
#
723-
# This param is required when creating cross-repository pull requests if both repositories
724-
# are owned by the same organization.
725-
#
726-
# https://github.com/sigmavirus24/github3.py/issues/1190
727-
728-
gh_pr: requests.Response = gh_app._post( # pylint: disable=W0212
729-
f"https://api.github.com/repos/{dest.ns}/{dest.name}/pulls",
730-
data={
731-
"title": title,
732-
"head": rebase.branch,
733-
"head_repo": f"{rebase.ns}/{rebase.name}",
734-
"base": dest.branch,
735-
"maintainer_can_modify": False,
736-
},
737-
json=True,
722+
dest_repo = gh_app.get_repo(f"{dest.ns}/{dest.name}")
723+
724+
pr = dest_repo.create_pull(
725+
title=title,
726+
body="",
727+
head=f"{rebase.ns}:{rebase.branch}",
728+
base=dest.branch,
729+
maintainer_can_modify=False,
738730
)
739731

740-
logging.debug(gh_pr.json())
741-
gh_pr.raise_for_status()
732+
logging.info(f"Created pull request: {pr.html_url}")
742733

743-
return gh_pr.json()["html_url"]
734+
return pr.html_url
744735

745736

746737
def is_ref_a_tag(gitwd: git.Repo, ref: str) -> bool:
@@ -867,12 +858,12 @@ def _init_working_dir(
867858
return gitwd
868859

869860

870-
def _manual_rebase_pr_in_repo(repo: Repository) -> ShortPullRequest | None:
861+
def _manual_rebase_pr_in_repo(repo: Repository) -> PullRequest | None:
871862
"""Checks for the presence of a rebase/manual label on the pull request."""
872-
prs = repo.pull_requests()
863+
prs = repo.get_pulls()
873864
for pull_req in prs:
874865
for label in pull_req.labels:
875-
if label["name"] == "rebase/manual":
866+
if label.name == "rebase/manual":
876867
return pull_req
877868
return None
878869

@@ -885,7 +876,7 @@ def _push_rebase_branch(gitwd: git.Repo, rebase: GitHubBranch) -> None:
885876
raise builtins.Exception(f"Error pushing to {rebase}: {result[0].summary}")
886877

887878

888-
def _update_pr_title(gitwd: git.Repo, pull_req: ShortPullRequest, source: GitHubBranch, dest: GitHubBranch) -> None:
879+
def _update_pr_title(gitwd: git.Repo, pull_req: PullRequest, source: GitHubBranch, dest: GitHubBranch) -> None:
889880
"""Updates the pull request title to match the current state of the rebase branch
890881
Only updates the title if the title contains the word Merge.
891882
Keeping everything before "Merge" and updating everything after.
@@ -902,8 +893,10 @@ def _update_pr_title(gitwd: git.Repo, pull_req: ShortPullRequest, source: GitHub
902893
return
903894

904895
logging.info(f"Updating pull request title: {computed_title}")
905-
if not pull_req.update(title=computed_title):
906-
raise PullRequestUpdateException(f"Error updating title for pull request: {pull_req.html_url}")
896+
try:
897+
pull_req.edit(title=computed_title)
898+
except Exception as ex:
899+
raise PullRequestUpdateException(f"Error updating title for pull request: {pull_req.html_url}") from ex
907900
else:
908901
logging.info(
909902
f'Open pull request title "{pull_req.title}" does not match rebasebot format. Keeping the current title.'
@@ -979,11 +972,11 @@ def run(
979972
hooks = lifecycle_hooks.LifecycleHooks(tmp_script_dir=None, args=None)
980973

981974
try:
982-
dest_repo = gh_app.repository(dest.ns, dest.name)
975+
dest_repo = gh_app.get_repo(f"{dest.ns}/{dest.name}")
983976
logging.info("Destination repository is %s", dest_repo.clone_url)
984-
rebase_repo = gh_cloner_app.repository(rebase.ns, rebase.name)
977+
rebase_repo = gh_cloner_app.get_repo(f"{rebase.ns}/{rebase.name}")
985978
logging.info("rebase repository is %s", rebase_repo.clone_url)
986-
source_repo = gh_app.repository(source.ns, source.name)
979+
source_repo = gh_app.get_repo(f"{source.ns}/{source.name}")
987980
logging.info("source repository is %s", source_repo.clone_url)
988981

989982
if not ignore_manual_label:
@@ -1171,9 +1164,11 @@ def run(
11711164
f"{ex}",
11721165
)
11731166
return False
1174-
except requests.exceptions.HTTPError as ex:
1175-
logging.error(f"Failed to create a pull request: {ex}\n Response: %s", ex.response.text)
1176-
_message_slack(slack_webhook, f"Failed to create a pull request: {ex}\n Response: {ex.response.text}")
1167+
except GithubException as ex:
1168+
logging.error(f"Failed to create a pull request: {ex}")
1169+
if ex.data:
1170+
logging.error(f"Response data: {ex.data}")
1171+
_message_slack(slack_webhook, f"Failed to create a pull request: {ex}")
11771172

11781173
return False
11791174
except Exception as ex:

rebasebot/cli.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -383,8 +383,8 @@ def main():
383383
"""Rebase Bot entry point function."""
384384
args = _parse_cli_arguments()
385385

386-
# Silence info logs from github3
387-
logger = logging.getLogger("github3")
386+
# Silence info logs from PyGithub
387+
logger = logging.getLogger("github")
388388
logger.setLevel(logging.WARN)
389389

390390
slack_webhook = None

rebasebot/github.py

Lines changed: 78 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,13 @@
1313
# under the License.
1414
"""Contains GitHub related helper classes."""
1515

16-
import builtins
1716
import logging
1817
import re
1918
from dataclasses import dataclass
2019
from functools import cached_property
2120
from urllib.parse import urlparse
2221

23-
import github3
22+
from github import Auth, Github, GithubIntegration, UnknownObjectException
2423

2524
logger = logging.getLogger()
2625

@@ -119,6 +118,9 @@ def __init__(
119118
self._app_credentials = None
120119
self._cloner_app_credentials = None
121120

121+
if self.user_auth and not self.user_token:
122+
raise ValueError("User authentication requires a GitHub user token")
123+
122124
if not user_auth:
123125
if not all((app_id, app_key, dest_branch, cloner_id, cloner_key, rebase_branch)):
124126
raise ValueError("Credentials for both, cloning and pushing app should be provided")
@@ -135,67 +137,122 @@ def get_app_token(self) -> str:
135137
136138
:return: str
137139
"""
138-
return self.github_app.session.auth.token
140+
if self.user_auth:
141+
return self._require_user_token()
142+
143+
return self._get_installation_token(self._app_installation_context, "app")
139144

140145
def get_cloner_token(self) -> str:
141146
"""
142147
Get cloner app auth token
143148
144149
:return: str
145150
"""
146-
return self.github_cloner_app.session.auth.token
151+
if self.user_auth:
152+
return self._require_user_token()
153+
154+
return self._get_installation_token(self._cloner_app_installation_context, "cloner")
147155

148156
@cached_property
149-
def github_app(self) -> github3.GitHub:
157+
def github_app(self) -> Github:
150158
"""
151159
Authenticated GitHub app.
152160
153161
In case `user_auth` = True, returns app authenticated with user token.
154162
In app mode `app_id`, `app_key` and `dest_branch` will be used for app authentication.
155163
156-
:return: github3.GitHub
164+
:return: Github
157165
"""
158166
if self.user_auth:
159167
return self._get_github_user_logged_in_app()
160168

161-
return self._github_login_app(self._app_credentials)
169+
return self._get_github_installation_app(self._app_installation_context)
162170

163171
@cached_property
164-
def github_cloner_app(self) -> github3.GitHub:
172+
def github_cloner_app(self) -> Github:
165173
"""
166174
Authenticated GitHub app.
167175
168176
In case `user_auth` = True, returns app authenticated with user token.
169177
In app mode `cloner_id`, `cloner_key` and `rebase_branch`will be used for app authentication.
170178
171-
:return: github3.GitHub
179+
:return: Github
172180
"""
173181
if self.user_auth:
174182
return self._get_github_user_logged_in_app()
175183

176-
return self._github_login_app(self._cloner_app_credentials)
184+
return self._get_github_installation_app(self._cloner_app_installation_context)
185+
186+
@cached_property
187+
def _app_installation_context(self) -> tuple[GithubIntegration, int]:
188+
return self._get_installation_context(self._require_credentials(self._app_credentials, "app"))
189+
190+
@cached_property
191+
def _cloner_app_installation_context(self) -> tuple[GithubIntegration, int]:
192+
return self._get_installation_context(self._require_credentials(self._cloner_app_credentials, "cloner"))
193+
194+
def _require_user_token(self) -> str:
195+
if self.user_token is None:
196+
raise RuntimeError("GitHub user token is unavailable")
197+
return self.user_token
177198

178199
@staticmethod
179-
def _github_login_app(credentials: GitHubAppCredentials) -> github3.GitHub:
200+
def _require_credentials(credentials: GitHubAppCredentials | None, role: str) -> GitHubAppCredentials:
201+
if credentials is None:
202+
raise RuntimeError(f"GitHub {role} credentials are unavailable")
203+
return credentials
204+
205+
@staticmethod
206+
def _get_github_installation_app(installation_context: tuple[GithubIntegration, int]) -> Github:
207+
"""
208+
Build a Github client authenticated as a GitHub App installation.
209+
210+
:return: Github
211+
"""
212+
integration, installation_id = installation_context
213+
return integration.get_github_for_installation(installation_id)
214+
215+
@staticmethod
216+
def _get_installation_token(installation_context: tuple[GithubIntegration, int], role: str) -> str:
217+
"""
218+
Fetch a fresh access token for a GitHub App installation.
219+
220+
:return: access token
221+
"""
222+
integration, installation_id = installation_context
223+
token = integration.get_access_token(installation_id).token
224+
if not token:
225+
raise RuntimeError(f"GitHub {role} token is unavailable")
226+
return token
227+
228+
@staticmethod
229+
def _get_installation_context(credentials: GitHubAppCredentials) -> tuple[GithubIntegration, int]:
230+
"""
231+
Authenticate as a GitHub App and locate its installation for a repository.
232+
233+
:return: tuple of (GithubIntegration, installation_id)
234+
"""
180235
logging.info("Logging to GitHub as an Application for repository %s", credentials.github_branch.url)
181-
gh_app = github3.GitHub()
182-
gh_app.login_as_app(credentials.app_key, credentials.app_id, expire_in=300)
183236
gh_branch = credentials.github_branch
184237

238+
# Create app authentication
239+
app_auth = Auth.AppAuth(credentials.app_id, credentials.app_key.decode("utf-8"))
240+
gi = GithubIntegration(auth=app_auth)
241+
185242
try:
186-
install = gh_app.app_installation_for_repository(owner=gh_branch.ns, repository=gh_branch.name)
187-
except github3.exceptions.NotFoundError as err:
243+
# Get installation for the repository
244+
installation = gi.get_repo_installation(gh_branch.ns, gh_branch.name)
245+
except UnknownObjectException as err:
188246
msg = (
189247
f"App has not been authorized by {gh_branch.ns}, or repo {gh_branch.ns}/{gh_branch.name} does not exist"
190248
)
191249
logging.error(msg)
192-
raise builtins.Exception(msg) from err
250+
raise RuntimeError(msg) from err
193251

194-
gh_app.login_as_app_installation(credentials.app_key, credentials.app_id, install.id)
195-
return gh_app
252+
return gi, installation.id
196253

197-
def _get_github_user_logged_in_app(self) -> github3.GitHub:
254+
def _get_github_user_logged_in_app(self) -> Github:
198255
logging.info("Logging to GitHub as a User")
199-
gh_app = github3.GitHub()
200-
gh_app.login(token=self.user_token)
256+
auth = Auth.Token(self._require_user_token())
257+
gh_app = Github(auth=auth)
201258
return gh_app

0 commit comments

Comments
 (0)