You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Potential Webhook Race Condition causes too many reviews to be requested.
The opened event sometimes has no users in it. This opened event causes two more request events to be be generated as Git Slackin requests X more reviews to hit the required number.
Reproduction
Click Open PR
Request reviews from 2 people (if 2 is the required number of reviewers for you)
Open PR
Git Slackin will receive 3 webhook events:
PR Opened
Review Requested 1
Review Requested 2
The PR Opened event will cause Git Slackin' to look for 2 random folks to review causing 2-4 (because it may overlap with the ones manually assigned) Reviewers to be requested.
Solution
This could be fixed by putting a setTimeout after receiving an opened PR event for a few seconds. Generally it looks like within 1 second all the events will come in, so call it 4 seconds to be safe?
I can use the PR as the unique identifier so we can cancel/reset the timeout if necessary.
This may be a good time to try implementing #31 as well?
The text was updated successfully, but these errors were encountered:
Potential Webhook Race Condition causes too many reviews to be requested.
The opened event sometimes has no users in it. This opened event causes two more request events to be be generated as Git Slackin requests X more reviews to hit the required number.
Reproduction
Git Slackin will receive 3 webhook events:
The
PR Opened
event will cause Git Slackin' to look for 2 random folks to review causing 2-4 (because it may overlap with the ones manually assigned) Reviewers to be requested.Solution
This could be fixed by putting a setTimeout after receiving an opened PR event for a few seconds. Generally it looks like within 1 second all the events will come in, so call it 4 seconds to be safe?
I can use the PR as the unique identifier so we can cancel/reset the timeout if necessary.
This may be a good time to try implementing #31 as well?
The text was updated successfully, but these errors were encountered: