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

Enhance setup-python Action in GitHub Enterprise Server with Raw API Fallback Mechanism #756

Closed
Shegox opened this issue Oct 31, 2023 · 6 comments · Fixed by #766
Closed
Assignees
Labels
feature request New feature or request to improve the current logic

Comments

@Shegox
Copy link
Contributor

Shegox commented Oct 31, 2023

Description:

In the context of GitHub Enterprise Server (GHES), it is typical for runners to share the same IP address due to Network Address Translation (NAT). This configuration results in the quick exhaustion of the unauthenticated rate limit (60 requests per hour per IP address) when accessing the versions-manifest.json file, leading to failures in the setup-python action. The current workaround is to incorporate a "github.com" token into the setup-python action. Although this solution is functional, it necessitates the creation of an additional github.com token (and technical user) for each repository/team, which is not optimal. Moreover, Pull Requests from forks are not supported due to their lack of access to the secret.

A potential improvement could be leveraging the raw API to retrieve the version-manifest, as it does not impose a rate limit and hence facilitates unrestricted consumption without the need for a token.

Justification:
Our GitHub Enterprise Server has frequently encountered rate limit issues, and the manual token addition method does not scale well. Utilizing the raw API as a fallback could provide an automatic workaround for this issue. If this approach fails, users can still resort to adding a token.

Are you willing to submit a PR?
I have created a fork to verify that the raw API isn't affected by the rate limit and can be used as a fallback solution. (https://github.com/actions/setup-python/compare/main...Shegox:setup-python:raw-access?expand=1)

The log extract from the workaround is as follows:

  Version 3.10 was not found in the local cache
  ##[debug]Getting manifest from actions/python-versions@main
  ##[debug]{"name":"HttpClientError","statusCode":403,"result":{"message":"API rate limit exceeded for xx.xx.xx.xx. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)","documentation_url":"https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting"}}
  ##[debug]Fetching via the API failed. Fetching using raw URL.
  ##[debug]check 3.13.0-alpha.1 satisfies 3.10
  ##[debug]check 3.12.0 satisfies 3.10
  ##[debug]check 3.12.0-rc.3 satisfies 3.10
  ##[debug]check 3.12.0-rc.2 satisfies 3.10
  ##[debug]check 3.12.0-rc.1 satisfies 3.10

I am prepared to submit a PR incorporating the proposed changes.
Before proposing this PR I wanted to check if that would be accepted or if there are other ideas to overcome this?

Related Issues

@Shegox Shegox added feature request New feature or request to improve the current logic needs triage labels Oct 31, 2023
@dmitry-shibanov
Copy link
Contributor

Hello @Shegox. Thank you for your report. We'll take a look on it.

@Shegox
Copy link
Contributor Author

Shegox commented Nov 15, 2023

I had some more time today and created #766 as an implementation of this idea, would be happy about feedback and maybe getting this merged :)

@priya-kinthali priya-kinthali self-assigned this Feb 13, 2024
@priya-kinthali
Copy link
Contributor

Hello @Shegox 👋,
Thank you for your contribution.
We have incorporated your proposed changes into the setup-actions-demo/setup-python@main branch. Could you kindly verify if the issue related to the rate limit has been resolved and confirm that no other new issues have emerged!

If there are no new issues, we will proceed with merging the original PR and releasing it.

@Shegox
Copy link
Contributor Author

Shegox commented Feb 13, 2024

@priya-kinthali just checked it again and it works with the setup-actions-demo fork as well. I copied the action over to my local Enterprise Server and run it there. Here the log extract:

...
2024-02-13T10:08:42.2420050Z Version 3.10 was not found in the local cache
2024-02-13T10:08:42.2441726Z ##[debug]Getting manifest from actions/python-versions@main
2024-02-13T10:08:42.5637799Z ##[debug]Fetching the manifest via the API failed.
2024-02-13T10:08:42.5653476Z ##[debug]API rate limit exceeded for 130.214.230.45. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)
2024-02-13T10:08:42.5654612Z ##[debug]Falling back to fetching the manifest using raw URL.
2024-02-13T10:08:42.7485896Z ##[debug]check 3.13.0-alpha.3 satisfies 3.10
...
2024-02-13T10:08:42.7569872Z ##[debug]check 3.10.13 satisfies 3.10
2024-02-13T10:08:42.7570475Z ##[debug]x64===x64 && darwin===linux
2024-02-13T10:08:42.7571062Z ##[debug]x64===x64 && linux===linux
2024-02-13T10:08:42.7574872Z ##[debug]x64===x64 && linux===linux
2024-02-13T10:08:42.7576643Z ##[debug]matched 3.10.13
2024-02-13T10:08:42.7578167Z Version 3.10 is available for downloading
2024-02-13T10:08:42.7580119Z Download from "https://github.com/actions/python-versions/releases/download/3.10.13-5997403688/python-3.10.13-linux-22.04-x64.tar.gz"
2024-02-13T10:08:42.7677836Z ##[debug]Downloading https://github.com/actions/python-versions/releases/download/3.10.13-5997403688/python-3.10.13-linux-22.04-x64.tar.gz
2024-02-13T10:08:42.7678780Z ##[debug]Destination /home/ccloud/_work/_temp/d5034090-7a92-4dff-b26f-da425ba507cf
2024-02-13T10:08:44.1541033Z ##[debug]download complete
...

I checked as well that the current PR (#766) doesn't overwrite any changes to the transpiled dist. So from my side it is good to be merged. Looking forward to seeing it merged!

@Shegox
Copy link
Contributor Author

Shegox commented Mar 25, 2024

@priya-kinthali did you have time to check on this issue and the PR (#766)?
I merged origin/main again to ensure that the PR is up to date

@priya-kinthali
Copy link
Contributor

Hello @Shegox 👋,
We have completed the review of your changes in PR #766 and are excited to announce that they will be included in our upcoming release. Your contribution significantly improves our project and we truly appreciate your effort. We look forward to more collaborations in the future.
Thank you once again for your valuable contribution:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request to improve the current logic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants