Skip to content

Commit d527d2b

Browse files
committed
fix: address PR review feedback
## What/Why Address reviewer feedback on PR #118: remove custom retry/timeout constants in favor of PyGithub defaults, sanitize trailing slashes on GHE URLs, fix unnecessary split/rejoin patterns for get_repo() calls, and correct docstring/comment references. ## Proof it works All 283 tests pass. Pylint shows only pre-existing warnings. ## Risk + AI role Low -- cleanup changes only. AI-assisted (Claude Opus 4.6). ## Review focus - auth.py retry/timeout removal: verify PyGithub defaults are acceptable - ghe.rstrip("/") placement in both auth functions Signed-off-by: jmeridth <jmeridth@gmail.com>
1 parent 05b06a1 commit d527d2b

4 files changed

Lines changed: 10 additions & 32 deletions

File tree

auth.py

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,6 @@
11
"""This is the module that contains functions related to authenticating to GitHub with a personal access token."""
22

33
from github import Auth, Github, GithubIntegration
4-
from urllib3.util.retry import Retry
5-
6-
# Retry strategy: 5 retries with exponential backoff for transient errors
7-
RETRY_STRATEGY = Retry(
8-
total=5,
9-
backoff_factor=1, # 1s, 2s, 4s, 8s, 16s
10-
status_forcelist=[429, 500, 502, 503, 504],
11-
raise_on_status=False,
12-
)
13-
REQUEST_TIMEOUT = 30 # seconds
144

155

166
def auth_to_github(
@@ -35,35 +25,25 @@ def auth_to_github(
3525
Returns:
3626
Github: the GitHub connection object
3727
"""
28+
ghe = ghe.rstrip("/")
29+
3830
if gh_app_id and gh_app_private_key_bytes and gh_app_installation_id:
3931
app_auth = Auth.AppAuth(int(gh_app_id), gh_app_private_key_bytes.decode())
4032
installation_auth = app_auth.get_installation_auth(int(gh_app_installation_id))
4133
if ghe and gh_app_enterprise_only:
4234
github_connection = Github(
4335
base_url=f"{ghe}/api/v3",
4436
auth=installation_auth,
45-
retry=RETRY_STRATEGY,
46-
timeout=REQUEST_TIMEOUT,
4737
)
4838
else:
49-
github_connection = Github(
50-
auth=installation_auth,
51-
retry=RETRY_STRATEGY,
52-
timeout=REQUEST_TIMEOUT,
53-
)
39+
github_connection = Github(auth=installation_auth)
5440
elif ghe and token:
5541
github_connection = Github(
5642
base_url=f"{ghe}/api/v3",
5743
auth=Auth.Token(token),
58-
retry=RETRY_STRATEGY,
59-
timeout=REQUEST_TIMEOUT,
6044
)
6145
elif token:
62-
github_connection = Github(
63-
auth=Auth.Token(token),
64-
retry=RETRY_STRATEGY,
65-
timeout=REQUEST_TIMEOUT,
66-
)
46+
github_connection = Github(auth=Auth.Token(token))
6747
else:
6848
raise ValueError(
6949
"GH_TOKEN or the set of [GH_APP_ID, GH_APP_INSTALLATION_ID, "
@@ -93,6 +73,7 @@ def get_github_app_installation_token(
9373
str: the GitHub App token
9474
"""
9575
try:
76+
ghe = ghe.rstrip("/")
9677
app_auth = Auth.AppAuth(int(gh_app_id), gh_app_private_key_bytes.decode())
9778
if ghe:
9879
gi = GithubIntegration(auth=app_auth, base_url=f"{ghe}/api/v3")

pr_comment.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,7 @@ def post_pr_comments(
7070

7171
for repo_name in all_repo_names:
7272
conflicts = all_conflicts_by_repo.get(repo_name, [])
73-
owner, repo = repo_name.split("/")
74-
repo_obj = github_connection.get_repo(f"{owner}/{repo}")
73+
repo_obj = github_connection.get_repo(repo_name)
7574

7675
# Group active conflicts by PR
7776
pr_conflicts = group_conflicts_by_pr(conflicts) if conflicts else {}

pr_conflict_detector.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,7 @@ def main():
217217
print("Report issue creation disabled (ENABLE_REPORT_ISSUES=false)")
218218
elif not env_vars.dry_run:
219219
for repo_full_name, conflicts in all_conflicts.items():
220-
owner, rname = repo_full_name.split("/")
221-
repo_obj = github_connection.get_repo(f"{owner}/{rname}")
220+
repo_obj = github_connection.get_repo(repo_full_name)
222221
issue_url = create_or_update_issue(
223222
repo_obj, conflicts, env_vars.report_title, env_vars.dry_run
224223
)
@@ -284,8 +283,7 @@ def get_repos_iterator(github_connection, env_vars):
284283

285284
repos = []
286285
for repo_full_name in env_vars.repository_list:
287-
owner, repo_name = repo_full_name.split("/")
288-
repos.append(github_connection.get_repo(f"{owner}/{repo_name}"))
286+
repos.append(github_connection.get_repo(repo_full_name))
289287
return repos
290288

291289

pr_data.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ def get_pr_changed_files( # pylint: disable=unused-argument
9797
"""
9898
Fetch the list of changed files for a given pull request.
9999
100-
Uses the PyGithub pull request's files() method, then parses
100+
Uses the PyGithub pull request's get_files() method, then parses
101101
each file's patch to extract modified line ranges.
102102
103103
Args:
@@ -171,7 +171,7 @@ def fetch_all_pr_data(
171171
print(f" Progress: {i + 1}/{total} PRs processed")
172172

173173
try:
174-
# Re-fetch the full PR object to call files()
174+
# Re-fetch the full PR object to call get_files()
175175
full_pr = repo.get_pull(pr_data.number) # type: ignore[attr-defined]
176176
pr_data.changed_files = get_pr_changed_files(
177177
full_pr, github_connection, owner, repo_name

0 commit comments

Comments
 (0)