fix: support adding comments for fork repo #78
Conversation
shenxianpeng
changed the title
shenxianpeng
changed the title
|
Will take a look this link to see if any solution https://github.com/search?q=PyGitHub+Resource+not+accessible+by+integration&type=issues |
|
github.GithubIntegration may help |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
WalkthroughThe pull request modifies the GitHub Actions workflow configuration by updating permissions in the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
.github/workflows/commit-check.yml (1)
12-14: Consider adding security documentation for elevated permissions.While the permission changes are necessary, it's recommended to document why these elevated permissions are required, especially for security auditing purposes.
Add a comment block explaining the permissions:
permissions: # use permissions because of use pr-comments + # Enhanced permissions required for commenting on forked repository PRs: + # - contents: write - Required for creating/updating PR comments + # - id-token: write - Required for authentication with forked repositories contents: write pull-requests: write id-token: writemain.py (1)
107-108: Remove commented-out code.The old initialization can be safely removed as it's tracked in version control.
- # g = Github(token) g = Github(token, retry=GithubRetry())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
.github/workflows/commit-check.yml(1 hunks)main.py(2 hunks)requirements.txt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- requirements.txt
🔇 Additional comments (3)
.github/workflows/commit-check.yml (1)
12-14: Permission changes look appropriate for handling forked repository comments.
The elevated permissions are necessary to resolve the 403 Forbidden error when posting comments on pull requests from forked repositories. The changes align with GitHub's documentation for handling cross-repository interactions.
Let's verify the minimal required permissions:
✅ Verification successful
Permission configuration is appropriate and consistent across workflows
The elevated permissions are justified and properly scoped:
- Both workflows use similar permission sets (
contents: write,pull-requests: write) - The additional
id-token: writein commit-check.yml is necessary for authentication with forked repos - Permissions align with their specific triggers:
- commit-check.yml runs on pull requests to main
- used-by.yml runs on schedule/manual dispatch
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any other workflows in the repository with similar permissions
# to ensure consistency and identify potential over-privileged workflows
# Search for workflow files with permission declarations
echo "Checking other workflow permissions..."
rg -l 'permissions:' .github/workflows/
Length of output: 191
Script:
#!/bin/bash
# Let's examine the permissions in both workflow files to compare them
echo "=== Permissions in commit-check.yml ==="
rg -A5 'permissions:' .github/workflows/commit-check.yml
echo -e "\n=== Permissions in used-by.yml ==="
rg -A5 'permissions:' .github/workflows/used-by.yml
# Also check the triggers to understand the context
echo -e "\n=== Workflow triggers in commit-check.yml ==="
rg "on:" -A5 .github/workflows/commit-check.yml
echo -e "\n=== Workflow triggers in used-by.yml ==="
rg "on:" -A5 .github/workflows/used-by.yml
Length of output: 1412
main.py (2)
6-6: LGTM! Good addition of retry mechanism.
Adding GithubRetry is a good approach to handle transient GitHub API errors, which could help with the 403 Forbidden errors on forked repositories.
107-108: Verify retry configuration with PyGithub 2.1.0.post0.
The code uses GithubRetry() with default settings. Let's verify the retry configuration works as expected with the downgraded PyGithub version.
✅ Verification successful
Let me gather more information about the GithubRetry usage and configuration.
GithubRetry configuration is compatible and properly implemented
The code correctly uses GithubRetry() with PyGithub 2.1.0.post0. The default retry configuration is appropriate for this use case as:
- The GithubRetry class is properly imported from the github module
- PyGithub 2.1.0.post0 is explicitly specified in requirements.txt
- The retry handler is correctly instantiated without custom parameters, which provides sensible defaults for retrying failed API requests
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify GithubRetry compatibility and configuration in PyGithub 2.1.0.post0
# Test 1: Check if GithubRetry is available in the installed version
pip freeze | grep -i pygithub
# Test 2: Search for GithubRetry usage patterns in the codebase
rg -l "GithubRetry"
# Test 3: Look for any retry-related configurations
rg "retry.*=|retries.*="
Length of output: 212
Script:
#!/bin/bash
# Check the imports and configuration of GithubRetry
rg -A 5 -B 5 "from github import.*GithubRetry"
# Look for any custom retry configurations or settings
rg -A 3 "GithubRetry\("
# Check requirements.txt or similar files for PyGithub version
fd "requirements.*\.txt$" --exec cat {}
Length of output: 764
| # g = Github(token) | ||
| g = Github(token, retry=GithubRetry()) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing error handling for specific GitHub API errors.
The current error handling catches all exceptions generically. Consider adding specific handling for GitHub API errors, especially for authentication and permission issues that are common with forked repositories.
g = Github(token, retry=GithubRetry())
repo = g.get_repo(repo_name)
pull_request = repo.get_issue(int(pr_number))
@@ -144,8 +144,17 @@
return 0 if result_text is None else 1
- except Exception as e:
- print(f"Error posting PR comment: {e}", file=sys.stderr)
- return 1
+ except github.GithubException as e:
+ if e.status == 403:
+ print(f"Permission denied: {e.data.get('message', '')}", file=sys.stderr)
+ print("This might be due to restricted permissions on forked repositories.", file=sys.stderr)
+ elif e.status == 404:
+ print(f"Resource not found: {e.data.get('message', '')}", file=sys.stderr)
+ else:
+ print(f"GitHub API error: {e}", file=sys.stderr)
+ return 1
+ except Exception as e:
+ print(f"Unexpected error: {e}", file=sys.stderr)
+ return 1Also applies to: 144-146
closes #77
Summary by CodeRabbit
New Features
Bug Fixes