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

fix: use authenticated github client when GITHUB_TOKEN env is available #456

Merged
merged 2 commits into from
Dec 5, 2022

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Dec 5, 2022

It seems that the culprit of #454 can be that we never checked the status code when querying Github for latest releases.

This patch changes the implementation to use the official github client which does account for rate limiting and properly return an error:

couldn't fetch latest knative/serving release: GET https://api.github.com/repos/knative/serving/releases/latest: 403 API rate limit exceeded for 89.69.160.71. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.) [rate reset in 27m47s]

Additionally this introduces context.Context whenever possible along the call path to allow cancellation to propagate.

Fixes #454

@pmalek pmalek requested a review from a team as a code owner December 5, 2022 16:12
@pmalek pmalek self-assigned this Dec 5, 2022
mlavacca
mlavacca previously approved these changes Dec 5, 2022
@pmalek pmalek requested a review from mlavacca December 5, 2022 16:24
@pmalek pmalek enabled auto-merge (squash) December 5, 2022 16:24
@pmalek pmalek merged commit f233236 into main Dec 5, 2022
@pmalek pmalek deleted the use-github-client-to-prevent-rate-limiting-in-tests branch December 5, 2022 16:35
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.

Knative addon tries to deploy with empty version even though set by default to 0.0.0
2 participants