Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 automatic module updates. Closes filecoin-station/roadmap#53 #316
Add automatic module updates. Closes filecoin-station/roadmap#53 #316
Changes from 15 commits
55ae0c5
ff1abde
42c2c8f
0a77c64
ec88b8b
77d7d48
8c241ab
8b6af85
5664075
4fd6900
4f63ae5
3c7d7c3
29b9beb
5e961ef
6c4cfb6
fe5347f
a9766ff
190a4cc
5db1171
8f5990a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Do you have any concerns about calling GitHub API anonymously? If there are more clients in the same network calling their API, then we will start seeing "rate limit exceeded" errors.
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.
(source: https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28_
Iiuc, this can only happen when there's many core instances on the same IP - which (for now) we don't want anyway.
I think otherwise we have to host our own updates api endpoint.
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.
Fair enough 👍🏻
So, what happens when Station Core hits this rate-limiting error? Is this something we should be concerned about?
Example: will it retry the requests and thus make the situation even worse?
I like that idea - the endpoint can send authenticated requests to GitHub, return cache-control (e.g. 1 minute?) and then we can use Cloudflare to shield us from too many requests.
Having written that, I think we don't need to add this extra complexity yet.
I am happy to keep things simple and use your current approach as long as we understand what will happen when we start hitting the rate limit errors, and we are okay with that.