Throwing an error in getManifestFromRepo when the github API rate limiting response is a JSON object.#945
Throwing an error in getManifestFromRepo when the github API rate limiting response is a JSON object.#945Lavaerius wants to merge 1 commit intoactions:mainfrom
Conversation
| } | ||
| } | ||
| return releases; | ||
| if (!releases.hasOwnProperty('documentation_url')){ |
There was a problem hiding this comment.
Dist/index.js is not where these changes should be directly made.
This index.js is a form of webpack and gets auto-generated from the source files in the src directory.
Line 13 in 70dcb22
Fundamentially you're trying to update the getManifestFromRepo function. That can be found here:
setup-python/src/install-python.ts
Lines 47 to 57 in 70dcb22
That uses a function from the our tool-cache NPM package in the actions/toolkit repository. This is the actual place where the change would need to be made: https://github.com/actions/toolkit/blob/6dd369c0e648ed58d0ead326cf2426906ea86401/packages/tool-cache/src/tool-cache.ts#L589-L632
So upstream the change would need to be made to tool-cache, a new version would then need to be published and then pulled into this repository/action. https://www.npmjs.com/package/@actions/tool-cache
I recommend submitting a PR to https://github.com/actions/toolkit
There was a problem hiding this comment.
Appreciate the feedback on this issue. I'll investigate your recommendation.
|
Hello @Lavaerius, Are you planning to make modifications to this PR as per @konradpabjan's suggestion? Thank you! |
|
Yes, I'll look into that. |
|
I've submitted: |
Description:
Updates fail condition for getManifestFromRepo function to include instances when github responds with JSON body for API rate limiting, that is incorrectly handled.
Related issue:
#903
Check list: