Copilot commented on code in PR #50533:
URL: https://github.com/apache/arrow/pull/50533#discussion_r3604495016
##########
dev/archery/archery/crossbow/core.py:
##########
@@ -452,6 +452,8 @@ def _github_login(self, github_token=None):
if not _have_github:
raise ImportError('Must install PyGithub')
github_token = github_token or self.github_token
+ if github_token is None:
+ return Github(timeout=30)
Review Comment:
This adds a new unauthenticated code path for `_github_login()` when no
token is provided; there are existing unit tests in `archery/crossbow/tests`,
but none appear to assert this behavior. Adding a small test that
`_github_login()` does not call `GithubAuth.Token` when the configured token is
`None` (and ideally when it's an empty string) would prevent regressions like
GH-50532.
##########
dev/archery/archery/crossbow/core.py:
##########
@@ -452,6 +452,8 @@ def _github_login(self, github_token=None):
if not _have_github:
raise ImportError('Must install PyGithub')
github_token = github_token or self.github_token
+ if github_token is None:
+ return Github(timeout=30)
return Github(auth=GithubAuth.Token(github_token), timeout=30)
Review Comment:
`github_token` may be an empty string (e.g., when Click reads an empty env
var). In that case this code will still build `Auth.Token('')` and attempt
authenticated requests, which is unlikely to work and defeats the intended
unauthenticated fallback. Consider treating any falsy token as "no token" (and
update the docstring since the method may now return an unauthenticated client).
--
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]
