-
Notifications
You must be signed in to change notification settings - Fork 562
wait for branch deletion to complete #1973
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
WalkthroughA one-second delay was added to the pull request event handler in the backend controller. When a pull request is closed, the handler now waits before attempting to retrieve the branch name, allowing time for GitHub to delete the branch and avoid potential errors. Changes
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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.
PR Summary
Added a 1-second delay in backend/controllers/github.go
to handle race conditions between PR closure and branch deletion operations on GitHub's side.
- Introduced sleep delay in
handlePullRequestEvent
to prevent false errors when checking branch existence after PR closure - Sleep is specifically added for 'closed' PR actions to allow GitHub's branch deletion to complete
- This change addresses a race condition where branch existence check could fail unnecessarily
1 file reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
// we sleep for 1 second to give github time to delete the branch | ||
time.Sleep(1 * time.Second) |
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.
style: This introduces a hardcoded delay. Consider making this configurable via env var.
// we sleep for 1 second to give github time to delete the branch | ||
time.Sleep(1 * time.Second) |
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.
style: sleep inside webhook handler could delay processing multiple events. Consider moving PR closure validation to async job.
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 (1)
backend/controllers/github.go (1)
449-450
: Potential flakiness from fixed sleep duration
A hard-coded 1s delay may not cover slower GitHub branch-deletion latencies and could introduce flakiness. Consider retrying the branch lookup with exponential backoff or making the delay configurable via an environment variable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/controllers/github.go
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
🔇 Additional comments (1)
backend/controllers/github.go (1)
20-20
: Add time import
Importing the “time” package is necessary to usetime.Sleep
. This change is correct and has no side effects.
Summary by CodeRabbit