Copilot commented on code in PR #48886:
URL: https://github.com/apache/arrow/pull/48886#discussion_r3393648199
##########
dev/archery/archery/crossbow/core.py:
##########
@@ -560,13 +534,14 @@ def github_overwrite_release_assets(self, tag_name,
target_commitish,
# remove the whole release if it already exists
try:
- release = repo.release_from_tag(tag_name)
- except github3.exceptions.NotFoundError:
- pass
- else:
- release.delete()
-
- release = repo.create_release(tag_name, target_commitish)
+ release = repo.get_release(tag_name)
+ release.delete_release()
+ except GithubException as e:
+ if e.status != 404:
+ raise
Review Comment:
Same issue as `github_release`: `repo.get_release(tag_name)` uses the
release-ID endpoint in `PyGithub`. If `tag_name` is a tag string, this should
use a tag-based lookup (e.g., `get_release_by_tag(tag_name)`) before deleting.
##########
dev/archery/archery/crossbow/core.py:
##########
@@ -448,110 +447,85 @@ def file_contents(self, commit_id, file):
blob = self.repo[entry.id]
return blob.data
- def _github_login(self, github_token):
- """Returns a logged in github3.GitHub instance"""
- if not _have_github3:
- raise ImportError('Must install github3.py')
+ def _github_login(self, github_token=None):
+ """Returns a logged in Github instance using PyGithub"""
+ if not _have_github:
+ raise ImportError('Must install PyGithub')
github_token = github_token or self.github_token
- session = github3.session.GitHubSession(
- default_connect_timeout=10,
- default_read_timeout=30
- )
- github = github3.GitHub(session=session)
- github.login(token=github_token)
- return github
+ return Github(auth=GithubAuth.Token(github_token), timeout=30)
def as_github_repo(self, github_token=None):
"""Converts it to a repository object which wraps the GitHub API"""
if self._github_repo is None:
github = self._github_login(github_token)
username, reponame = _parse_github_user_repo(self.remote_url)
- self._github_repo = github.repository(username, reponame)
+ self._github_repo = github.get_repo(f"{username}/{reponame}")
return self._github_repo
def token_expiration_date(self, github_token=None):
"""Returns the expiration date for the github_token provided"""
github = self._github_login(github_token)
- # github3 hides the headers from us. Use the _get method
- # to access the response headers.
- resp = github._get(github.session.base_url)
- # Response in the form '2023-01-23 10:40:28 UTC'
- date_string = resp.headers.get(
- 'github-authentication-token-expiration')
+ # PyGithub doesn't expose the token expiration header through a
+ # dedicated API, so request it via the public Requester escape hatch.
+ headers, _ = github.requester.requestJsonAndCheck("GET", "/user")
+ # Response header in the form '2023-01-23 10:40:28 UTC'
+ date_string = headers.get('github-authentication-token-expiration')
if date_string:
return date.fromisoformat(date_string.split()[0])
+ return None
def github_commit(self, sha):
repo = self.as_github_repo()
- return repo.commit(sha)
+ return repo.get_commit(sha)
def github_release(self, tag):
repo = self.as_github_repo()
try:
- return repo.release_from_tag(tag)
- except github3.exceptions.NotFoundError:
- return None
-
- def github_upload_asset_requests(self, release, path, name, mime,
- max_retries=None, retry_backoff=None):
+ return repo.get_release(tag)
+ except GithubException as e:
+ if e.status == 404:
+ return None
+ raise
Review Comment:
`PyGithub`'s `Repository.get_release(...)` expects a numeric release ID, not
a tag name. For tag-based lookups this should use the tag-specific API (e.g.,
`get_release_by_tag(tag)`), otherwise this will fail or behave incorrectly at
runtime.
##########
dev/archery/archery/crossbow/core.py:
##########
@@ -448,110 +447,85 @@ def file_contents(self, commit_id, file):
blob = self.repo[entry.id]
return blob.data
- def _github_login(self, github_token):
- """Returns a logged in github3.GitHub instance"""
- if not _have_github3:
- raise ImportError('Must install github3.py')
+ def _github_login(self, github_token=None):
+ """Returns a logged in Github instance using PyGithub"""
+ if not _have_github:
+ raise ImportError('Must install PyGithub')
github_token = github_token or self.github_token
- session = github3.session.GitHubSession(
- default_connect_timeout=10,
- default_read_timeout=30
- )
- github = github3.GitHub(session=session)
- github.login(token=github_token)
- return github
+ return Github(auth=GithubAuth.Token(github_token), timeout=30)
def as_github_repo(self, github_token=None):
"""Converts it to a repository object which wraps the GitHub API"""
if self._github_repo is None:
github = self._github_login(github_token)
username, reponame = _parse_github_user_repo(self.remote_url)
- self._github_repo = github.repository(username, reponame)
+ self._github_repo = github.get_repo(f"{username}/{reponame}")
return self._github_repo
def token_expiration_date(self, github_token=None):
"""Returns the expiration date for the github_token provided"""
github = self._github_login(github_token)
- # github3 hides the headers from us. Use the _get method
- # to access the response headers.
- resp = github._get(github.session.base_url)
- # Response in the form '2023-01-23 10:40:28 UTC'
- date_string = resp.headers.get(
- 'github-authentication-token-expiration')
+ # PyGithub doesn't expose the token expiration header through a
+ # dedicated API, so request it via the public Requester escape hatch.
+ headers, _ = github.requester.requestJsonAndCheck("GET", "/user")
+ # Response header in the form '2023-01-23 10:40:28 UTC'
+ date_string = headers.get('github-authentication-token-expiration')
if date_string:
return date.fromisoformat(date_string.split()[0])
+ return None
def github_commit(self, sha):
repo = self.as_github_repo()
- return repo.commit(sha)
+ return repo.get_commit(sha)
def github_release(self, tag):
repo = self.as_github_repo()
try:
- return repo.release_from_tag(tag)
- except github3.exceptions.NotFoundError:
- return None
-
- def github_upload_asset_requests(self, release, path, name, mime,
- max_retries=None, retry_backoff=None):
+ return repo.get_release(tag)
+ except GithubException as e:
+ if e.status == 404:
+ return None
+ raise
+
+ def github_upload_asset(self, release, path, name, mime,
+ max_retries=None, retry_backoff=None):
if max_retries is None:
max_retries = int(os.environ.get('CROSSBOW_MAX_RETRIES', 8))
if retry_backoff is None:
retry_backoff = int(os.environ.get('CROSSBOW_RETRY_BACKOFF', 5))
for i in range(max_retries):
try:
- with open(path, 'rb') as fp:
- result = release.upload_asset(name=name, asset=fp,
- content_type=mime)
- except github3.exceptions.ResponseError as e:
+ result = release.upload_asset(path, name=name,
+ content_type=mime)
+ logger.info(f"Attempt {i + 1} has finished.")
+ return result
+ except GithubException as e:
logger.error(f"Attempt {i + 1} has failed with message: {e}.")
- logger.error(f"Error message {e.msg}")
- logger.error("List of errors provided by GitHub:")
- for err in e.errors:
- logger.error(f" - {err}")
+ if hasattr(e, 'data'):
+ logger.error(f"Error data: {e.data}")
- if e.code == 422:
+ if e.status == 422:
# 422 Validation Failed, probably raised because
# ReleaseAsset already exists, so try to remove it before
# reattempting the asset upload
- for asset in release.assets():
+ for asset in release.get_assets():
if asset.name == name:
logger.info(f"Release asset {name} already exists,
"
"removing it...")
- asset.delete()
+ asset.delete_asset()
logger.info(f"Asset {name} removed.")
break
- except github3.exceptions.ConnectionError as e:
+ except IOError as e:
+ # Catch network and file I/O errors (includes requests
exceptions)
logger.error(f"Attempt {i + 1} has failed with message: {e}.")
Review Comment:
`requests` / transport-level failures raised by `PyGithub` are typically
`requests.exceptions.RequestException` (or other non-`IOError` exceptions), so
the retry loop may not trigger for common network failures. Update the
exception handling to catch the appropriate request/network exception type(s)
(and keep file errors as `OSError`/`IOError` as needed); also adjust the
comment since `IOError` does not generally include requests exceptions.
##########
ci/conda_env_archery.txt:
##########
@@ -19,7 +19,6 @@
click
# bot, crossbow
-github3.py
jinja2
jira
pygit2
Review Comment:
This environment file removes `github3.py` but does not add `pygithub`.
Since Crossbow now imports `github` from `PyGithub`, the Archery conda
environment will likely be missing a required dependency when Crossbow
functionality/tests run under this environment. Add `pygithub` here as well
(mirroring `ci/conda_env_crossbow.txt`) if this env is expected to support
Crossbow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
