-
-
Notifications
You must be signed in to change notification settings - Fork 2
fix: eliminate deprecation warnings during GitHub authentication #145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughUpdates Changes
Sequence Diagram(s)sequenceDiagram
actor GH as GitHub Action Runner
participant Script as main.py
participant PyGh as PyGithub
participant API as GitHub API
GH->>Script: Run PR commenting task
Script->>Script: Read GITHUB_TOKEN
alt Token missing
Script-->>GH: Raise ValueError
else Token present
Script->>PyGh: Github(auth=Auth.Token(token)) [New Auth API]
Note right of PyGh: Avoids deprecated login_or_token
Script->>API: Fetch repo and PR
Script->>API: Compute/read existing PR comments
Script->>API: Create/Update comment(s)
Script->>API: Delete older matching comments
API-->>Script: Responses
Script-->>GH: Completed
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Pre-merge checks (5 passed)✅ Passed checks (5 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews: pre_merge_checks: custom_checks: - name: "Undocumented Breaking Changes" mode: "warning" instructions: | Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Commit-Check ✔️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a deprecation warning by updating the GitHub API client initialization to use the new Auth API instead of the deprecated direct token parameter method.
- Updates the GitHub client initialization to use
Auth.Token()for authentication - Adds import for the
Authclass from the github module - Includes additional validation to ensure the GitHub token is set
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@coderabbitai
coderabbitai
bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
main.py (3)
6-6: Drop broad type ignore on PyGithub import.The Auth import is right for the deprecation fix; the inline
# type: ignoreis unnecessary here and suppresses real typing issues.-from github import Github, Auth # type: ignore +from github import Github, Auth
127-131: Auth.Token migration looks good; avoid catching your own guard and potentially failing the job.Raising
ValueErrorinside the surroundingtryis caught by the broadexcept, which can fail the workflow even when commit-check itself passed (also flagged by Ruff TRY301/TRY003). If PR comments are best-effort, early-return and reuse the module-level env var.- token = os.getenv("GITHUB_TOKEN") + token = GITHUB_TOKEN @@ - # Use new Auth API to avoid deprecation warning - if not token: - raise ValueError("GITHUB_TOKEN is not set") + # Use new Auth API to avoid deprecation warning + if not token: + print("GITHUB_TOKEN is not set; skipping PR comments.", file=sys.stderr) + return 0 g = Github(auth=Auth.Token(token))Would you prefer comment-post failures to be non-blocking (recommended), or should they fail the job?
119-125: Harden PR number extraction for non-PR refs.When
PR_COMMENTS="true"but the event is not a PR,GITHUB_REF.split('/')[-2]can become"heads"andint()later throws. Guard forrefs/pull/and skip gracefully.- pr_number = os.getenv("GITHUB_REF") - if pr_number is not None: - pr_number = pr_number.split("/")[-2] - else: - # Handle the case where GITHUB_REF is not set - raise ValueError("GITHUB_REF environment variable is not set") + ref = os.getenv("GITHUB_REF") + if not ref or "refs/pull/" not in ref: + print("Not a pull_request ref; skipping PR comments.", file=sys.stderr) + return 0 + pr_number = ref.split("/")[2]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
main.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
main.py
129-129: Abstract raise to an inner function
(TRY301)
129-129: Avoid specifying long messages outside the exception class
(TRY003)
Uh oh!
There was an error while loading. Please reload this page.
fixes #144
Summary by CodeRabbit
Bug Fixes
Chores