CI(Preview): add retries to find PR script #20
Conversation
|
Welcome to new-connect! Make sure to:
deployed preview: https://20.connect-d5y.pages.dev |
|
Meant to run this PR on my fork to see if it fixed the secondary rate limit problem I was hitting with my other PRs. My bad |
ugtthis
changed the title
.github/workflows/preview.yaml
Outdated
| const pullRequestNumber = items[0].number | ||
| console.info("Pull request number is", pullRequestNumber) | ||
| return pullRequestNumber | ||
| return findPR(); |
There was a problem hiding this comment.
Is there a reason to wrap this in a function?
There was a problem hiding this comment.
Maybe wrapping it in a function is not needed here, but the intention was to make it easier to manage, to debug, and easier to simulate different scenarios to make sure the retry logic works as expected. But maybe it’s overkill? Let me know if you prefer it a certain way
There was a problem hiding this comment.
I think it's better to just set retries on the action, it will handle the Retry-After header for us
https://docs.github.com/en/rest/using-the-rest-api/best-practices-for-using-the-rest-api#handle-rate-limit-errors-appropriately
There was a problem hiding this comment.
Gotcha. Thanks for the fix!
incognitojam
changed the title
To my understanding this is so pose to help by increasing the rate limit since I added the Github Token. I also added a retry mechanism instead of trying to rerun actions manually. I noticed that Cameron also ran into the secondary rate limit so hopefully this would help us not hit that limit anymore.
I'm not entirely confident this would help so feel free to close this. My previous PRs are now able to generate the Preview Link.
Reference Link: docs.github.com