Skip to content

refactor: migrate from github3-py to PyGithub#118

Merged
jmeridth merged 3 commits into
mainfrom
migrate-to-pygithub
Jul 1, 2026
Merged

refactor: migrate from github3-py to PyGithub#118
jmeridth merged 3 commits into
mainfrom
migrate-to-pygithub

Conversation

@jmeridth

@jmeridth jmeridth commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

What/Why

Replace github3-py with PyGithub (>=2.6.0) as the GitHub API client library, consolidating on a single actively maintained library across OSPO actions.

Proposed Changes

  • pyproject.toml: Swap github3-py for PyGithub>=2.6.0, drop explicit cryptography and pyjwt deps (PyGithub handles JWT internally)
  • auth.py: Rewrite to use Auth.Token(), Auth.AppAuth(), GithubIntegration; remove custom RETRY_STRATEGY/REQUEST_TIMEOUT constants and session monkey-patching (_configure_session(), _timeout_wrapper()) in favor of PyGithub's built-in defaults (GithubRetry(total=10), timeout=15)
  • Source files (conflict_detector.py, pr_comment.py, pr_data.py, issue_writer.py, pr_conflict_detector.py): Method renames -- repository() to get_repo(), pull_request() to get_pull(), pull_requests() to get_pulls(), issue_comments() to get_issue_comments(), create_comment() to create_issue_comment(), organization() to get_organization(), repositories() to get_repos(), files() to get_files(), team_by_name() to get_team_by_slug(), members() to get_members()
  • All test files: Updated mock patterns to match new PyGithub method names
  • .github/linters/.mypy.ini: github3.* to github.*

Proof it works

  • 283 tests pass with 100% code coverage
  • Tested locally with uv run pr_conflict_detector.py

Risk + AI role

Medium -- replaces the core GitHub API client across all modules. All source and test code AI-generated (Claude Opus 4.6).

Review focus

  • auth.py: Custom retry/timeout removed; PyGithub's built-in defaults (GithubRetry(total=10), timeout=15) handle transient errors and rate limits out of the box. Verify this is acceptable vs the old 5-retry/30s-timeout strategy.
  • Method rename correctness across source and test files.

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request
  • run make lint and fix any issues that you have introduced
  • run make test and ensure you have test coverage for the lines you are introducing

## What/Why

Replace github3-py with PyGithub (>=2.6.0) as the GitHub API client library,
aligning with the measure-innersource migration and consolidating on a
single actively maintained library across OSPO actions.

## Proof it works

283 tests pass with 100% code coverage. Black formatting verified clean.
Tested locally with `uv run pr_conflict_detector.py`.

## Risk + AI role

Medium -- replaces the core GitHub API client across all modules. All source
and test code generated by Claude Opus 4.6.

## Review focus

- auth.py: retry/timeout now passed via Github constructor kwargs instead of
  session monkey-patching. Verify the Retry strategy parameters match the
  original behavior.
- Method rename mappings: repository()->get_repo(), pull_request()->get_pull(),
  issue_comments()->get_issue_comments(), create_comment()->create_issue_comment(),
  organization()->get_organization(), repositories()->get_repos().

Signed-off-by: jmeridth <jmeridth@gmail.com>
@jmeridth jmeridth added the Mark Ready When Ready Automatically mark draft PR ready when checks pass label Jun 28, 2026
@jmeridth jmeridth self-assigned this Jun 28, 2026
@github-actions github-actions Bot added the automation CI/CD or automation changes label Jun 28, 2026
@github-actions
github-actions Bot marked this pull request as ready for review June 28, 2026 19:55
@github-actions
github-actions Bot requested a review from zkoppert as a code owner June 28, 2026 19:55
@github-actions github-actions Bot removed the Mark Ready When Ready Automatically mark draft PR ready when checks pass label Jun 28, 2026
@zkoppert
zkoppert requested a review from Copilot June 29, 2026 03:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates the action’s GitHub API client from github3-py to PyGithub, updating authentication and all API call sites/mocks accordingly to keep PR conflict detection, issue creation, and PR commenting working with the new client.

Changes:

  • Replace github3-py dependency with PyGithub>=2.6.0 and update lockfile accordingly.
  • Refactor authentication to use github.Auth/Github/GithubIntegration and pass retry/timeout via constructor kwargs.
  • Update all production call sites and unit tests for PyGithub method names (get_repo, get_pull, get_pulls, get_files, etc.) and adjust mypy ignore section.
Show a summary per file
File Description
uv.lock Updates locked dependencies to remove github3-py and add pygithub (+ transitive deps).
pyproject.toml Swaps github3-py/pyjwt/cryptography for PyGithub>=2.6.0.
auth.py Refactors authentication + app installation token flow to PyGithub APIs and constructor-based retry/timeout.
conflict_detector.py Migrates verify flow to get_repo()/get_pull() calls.
issue_writer.py Migrates issue lookup to repo.get_issues(...) and updates type-checking import.
pr_comment.py Migrates repo/PR/comment APIs to get_repo()/get_pull()/create_issue_comment().
pr_data.py Migrates PR listing + changed-files retrieval to get_pulls()/get_files()/get_pull().
pr_conflict_detector.py Migrates repository enumeration + issue creation repo lookup to PyGithub.
.github/linters/.mypy.ini Updates mypy ignore pattern from github3.* to github.*.
test_auth.py Updates auth tests to patch PyGithub classes and method names.
test_conflict_detector.py Updates mocks for get_repo()/get_pull() usage in verification.
test_issue_writer.py Updates mocks for get_issues() behavior.
test_pr_comment_integration.py Updates mocks for get_repo() call path.
test_pr_comment_posting.py Updates mocks for repo/PR comment APIs (get_pull, get_issue_comments, create_issue_comment).
test_pr_conflict_detector_exclude.py Updates organization/repo mocks to get_organization()/get_repos().
test_pr_conflict_detector_filtering.py Updates organization/repo mocks to get_organization()/get_repos().
test_pr_conflict_detector_flags.py Updates organization/repo mocks to get_organization()/get_repos().
test_pr_conflict_detector_main.py Updates repo/org mocks to get_repo()/get_organization().
test_pr_conflict_detector_repos.py Updates repo/org iterator tests to get_repo()/get_organization()/get_repos().
test_pr_data.py Updates PR data mocks to get_pulls()/get_pull()/get_files().

Review details

  • Files reviewed: 19/20 changed files
  • Comments generated: 5
  • Review effort level: Low

Comment thread auth.py
Comment thread auth.py
Comment thread auth.py
Comment thread pr_data.py Outdated
Comment thread pr_data.py Outdated
Comment thread auth.py
Comment thread auth.py Outdated
Comment thread pr_conflict_detector.py Outdated
Comment thread pr_comment.py Outdated
Comment thread pr_conflict_detector.py Outdated
## 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>

Copilot AI commented Jun 30, 2026

Copy link
Copy Markdown

@jmeridth Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub.

You can ask me to try again later by mentioning me in a new comment.

If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: 1e949ce1-d1b7-4891-9c77-d8a0401306a1

Sorry for the inconvenience!

@zkoppert zkoppert left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 19/20 changed files
  • Comments generated: 3
  • Review effort level: Low

Comment thread auth.py
Comment thread auth.py Outdated
Comment thread test_auth.py
The docstring documented 'str' as the return type but the function
signature is 'str | None' and returns None on failure.

Signed-off-by: jmeridth <jmeridth@gmail.com>
@jmeridth
jmeridth merged commit fbaa302 into main Jul 1, 2026
38 checks passed
@jmeridth
jmeridth deleted the migrate-to-pygithub branch July 1, 2026 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation CI/CD or automation changes release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants