-
Notifications
You must be signed in to change notification settings - Fork 129
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
fix: use retry and throttle octokit plugins #487
fix: use retry and throttle octokit plugins #487
Conversation
Refactors the client to use [@octokit/plugin-retry](https://www.npmjs.com/package/@octokit/plugin-retry) and [@octokit/plugin-throttling](https://www.npmjs.com/package/@octokit/plugin-throttling) as GitHub occasionally changes it's API and these plugins can abstract those changes away from this module. - Removes `lib/definitions/rate-limit.js` - Adds `lib/definitions/retry.js` and `lib/definitions/throttle.js` to handle retry/throttle settings - Updates tests to be more like GitHub (returing the correct rate limit response headers, etc) Fixes semantic-release#299 and semantic-release/semantic-release#2204 See also semantic-release#480 and semantic-release#378
I'm trialing this fix in a bunch of repos - so far, so good. If anyone else wants to give it a go you can use my temporary fork with this patch applied. Change your
No other changes required. |
It would be great if this PR can be reviewed and marged. I'm facing same problem in one of my projects |
Would love to get this reviewed and merged!! Can we update to resolve conflicts and tag some people for review? |
…-throttle-plugins
I'm still on parental leave, and first week back to work will be busy, but I have it on my radar |
@gr2m Wonderful to hear! Running into this same problem right now. All your hard work is much appreciated! Thanks! Please let me know if there is anything I can do to help 🙏❤️ |
@gr2m Any ETA for a review of this PR? We have to deal a lot with the secondary rate limits in our release process lately. |
No update yet, sorry. I'm still on parental leave and life keeps me busy. We focus the little time we have right now on #418 |
Hi @gr2m, our usage of We're also impacted by this problem now and then, is there a way we can help @gr2m? |
…the merge happens this semantic-release/github#487 pr is a fix to the issue where the github package hits a retry error. semantic release does publish though.
use semantic release github fork while semantic-release/github#487 is still waiting to be merged. the issue is semantic release times out in release on the github package but does publish. also fixes the error where I put only one file in the package.json. instead we copy the README.md from the root package into the package/ file before we publish
…the merge happens use semantic release github fork while semantic-release/github#487 is still waiting to be merged. the issue is semantic release times out in release on the github package but does publish. also fixes the error where I put only one file in the package.json. instead we copy the README.md from the root package into the package/ file before we publish
…the merge happens use semantic release github fork while semantic-release/github#487 is still waiting to be merged. the issue is semantic release times out in release on the github package but does publish. also fixes the error where I put only one file in the package.json. instead we copy the README.md from the root package into the package/ file before we publish
…-throttle-plugins
@travi @gr2m is there any chance you could find some time to review this PR? I've updated it to resolve the recent conflicts, it solves a very real problem when using this module in monorepos that publish lots of modules in one go. Please let me know if there's anything I can do to help you get it across the line? |
+1 -- we constantly run into rate-limiting issues in our delivery workflows. Getting quite tedious having to constantly monitor and manually retry failed actions. |
nx trips over the tarball dep we have on `@semantic-release/github` when it tries to parse project deps from the `package-lock.json` in CI so disable lockfiles in monorepos. This will no longer be necessary when semantic-release/github#487 is merged.
nx trips over the tarball dep we have on `@semantic-release/github` when it tries to parse project deps from the `package-lock.json` in CI so disable lockfiles in monorepos. This will no longer be necessary when semantic-release/github#487 is merged.
sorry for pinging directly but we really need traction on this PR to get it merged. It has been 8 months already and this issue is plaguing many people. Can one of you review and get this merged please? @boennemann @gr2m @kbrandwijk @keplersj @pvdlg @travi @UziTech |
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.
I'm no semantic-release expert but the rest LGTM
Okay more like May 12 but that's how life goes sometimes. Thank you all for your patience and your help. I'm looking into it now |
…ot exist on `@octokit/core`
It's happening dot gif If I had more time I'd add tests for I'm looking into the failing Node 14.17 tests. If y'all could give the latest version a try, I'd be much obliged |
lib/definitions/retry.js
Outdated
retries: 3, | ||
retryAfterBaseValue: 1000, |
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.
these are default values already set in Octokit.
I published the current version as You can use aliasing to test it out. In your package.json "devDependencies": {
"semantic-release": "^21.0.2",
"@semantic-release/github": "npm:@gr2m/semantic-release-github-487@0.0.0-development"
}, |
Alright I tested it some, it's Saturday, let's give this a go. I'm confident in this not introducing any new problems, but we can really only see once it's used out there. |
We want to thank you all for your patience and support, really 💐❤️ |
🎉 This PR is included in version 8.0.8 🎉 The release is available on: Your semantic-release bot 📦🚀 |
We keep getting this error from semantic-release: HttpError: You have exceeded a secondary rate limit. Please wait a few minutes before you try again. This is supposedly fixed in semantic-release/github#487.
We keep getting this error from semantic-release: HttpError: You have exceeded a secondary rate limit. Please wait a few minutes before you try again. This is supposedly fixed in semantic-release/github#487.
Refactors the client to use @octokit/plugin-retry and @octokit/plugin-throttling as GitHub occasionally changes it's API and these plugins can abstract those changes away from this module.
lib/definitions/rate-limit.js
lib/definitions/retry.js
andlib/definitions/throttle.js
to handle retry/throttle settingsFixes #299 and semantic-release/semantic-release#2204
See also #480 and #378