Skip to content
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

Add support for locally hosted GHES instances to reduce rate limiting #720

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

C1ARKGABLE
Copy link

@C1ARKGABLE C1ARKGABLE commented Aug 23, 2023

Description:
Rate limiting, while necessary, can be frustrating to deal with. The work around is not always a viable solution. No company wants to rely solely on a single employee's GitHub.com PAT for significant internal projects.
This attempts to resolve rate limiting errors by allowing users to internally host a copy of actions/python-versions and specify their GHES api endpoint instead of the GitHub.com endpoint being hardcoded.

##[error]API rate limit exceeded for YOUR_IP. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)

Related issue:
#683
#638
#666
#316

#443 (comment)

https://docs.github.com/en/enterprise-server/admin/github-actions/managing-access-to-actions-from-githubcom/setting-up-the-tool-cache-on-self-hosted-runners-without-internet-access

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

@C1ARKGABLE C1ARKGABLE requested a review from a team as a code owner August 23, 2023 22:37
@C1ARKGABLE
Copy link
Author

Guidance + suggestions are welcome!

@bhundven
Copy link

@e-korolevskii @dmitry-shibanov This also affects a GHE instance I use.

@@ -10957,10 +10957,10 @@ function findAllVersions(toolName, arch) {
return versions;
}
exports.findAllVersions = findAllVersions;
function getManifestFromRepo(owner, repo, auth, branch = 'master') {
function getManifestFromRepo(owner, repo, auth, branch = 'master', serverUrl = 'https://api.github.com') {
Copy link

@jhaxon jhaxon Oct 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getManifestFromRepo() is a function from a library imported at the top of src/install-python.ts. The code here in dist/* is generated from that library at compile time, so your changes will be overwritten.

For reference, here is the source of the library function. If you truly intend to reuse the library function you'll first need to merge a PR changing its signature here.

In order for this PR to work you need to limit your changes to the src/*.ts files.

@@ -10,7 +10,11 @@ const AUTH = !TOKEN ? undefined : `token ${TOKEN}`;
const MANIFEST_REPO_OWNER = 'actions';
const MANIFEST_REPO_NAME = 'python-versions';
const MANIFEST_REPO_BRANCH = 'main';
export const MANIFEST_URL = `https://raw.githubusercontent.com/${MANIFEST_REPO_OWNER}/${MANIFEST_REPO_NAME}/${MANIFEST_REPO_BRANCH}/versions-manifest.json`;
const API_URL = core.getInput('github_api_url');
const GITHUB_API_URL = API_URL ? 'https://api.github.com' : API_URL;
Copy link

@jhaxon jhaxon Oct 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

truthy and falsey expressions are backwards here, should be API_URL ? API_URL : ...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unit tests caught this! ;)

const API_URL = core.getInput('github_api_url');
const GITHUB_API_URL = API_URL ? 'https://api.github.com' : API_URL;
const RAW_URL = core.getInput('github_raw_url');
const GITHUB_RAW_URL = RAW_URL ? 'https://raw.githubusercontent.com' : RAW_URL;
Copy link

@jhaxon jhaxon Oct 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

truthy and falsey expressions are backwards here, should be RAW_URL ? RAW_URL : ...

@jhaxon
Copy link

jhaxon commented Oct 10, 2023

Without any changes to installCpythonFromRelease the action will download the release from github.com. Further work is needed here.

@C1ARKGABLE C1ARKGABLE marked this pull request as draft October 10, 2023 23:17
@C1ARKGABLE
Copy link
Author

Converted to a draft until I can work on this further

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants