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

Rate limit #6

Open
eps1lon opened this issue Apr 2, 2020 · 5 comments
Open

Rate limit #6

eps1lon opened this issue Apr 2, 2020 · 5 comments
Labels
wontfix This will not be worked on

Comments

@eps1lon
Copy link

eps1lon commented Apr 2, 2020

At first let me thank you for this awesome extension! Really levels up the GitHub experience.

I'm a bit concerned about the rate limit. It seems to send a request for every file that is viewed. Is this a limitation of the github API or is it possible to fetch files in batches (e.g. by directory) instead of fetching each file?

@brumm
Copy link
Owner

brumm commented Apr 2, 2020

Check here:

tako/src/api.js

Lines 7 to 11 in 3e697cc

const MAX_REQUESTS = 10
const limiter = new Bottleneck({
maxConcurrent: MAX_REQUESTS,
})
We're already using bottleneck to rate limit requests as per Github's recommendation :)

About batching—unfortunately Github does not offer any way to get the most recent commit of a whole folder, only per file. That's what all those requests are about. We're doing our best here to de-prioritize prefetches for the actual http request as well as the limiter batching, so they don't clog up the queue.

@brumm brumm closed this as completed Apr 2, 2020
@eps1lon
Copy link
Author

eps1lon commented Apr 2, 2020

We're already using bottleneck to rate limit requests as per Github's recommendation :)

You can't magically increase the maximum limit per hour though.

I think disabling the file viewer would help alot. Honestly I think I won't be using this extension if it keeps including the file preview. I like the treeview but the file preview is an additional concern that I don't think is useful.

The file preview is the feature responsible for the api requests per file, right? Could we disable that?

@brumm
Copy link
Owner

brumm commented Apr 2, 2020

Hm good point.

Each file or folder spawns a request to repos/${user}/${repo}/commits?page=1&per_page=1&sha=${branch}&path=${path} to get the latest commit message and date. That's the part we can't really do anything about save for disabling it.

Also, every file spawns a request for the file's contents after 200ms of hovering over the file name cell of the table.

I'd be open to accept a PR which adds support for disabling the file preview, but I'm not really interested in doing it myself at this point. In the end, rather than causing big changes for a potential issue, I'd rather wait and see whether anyone actually hits the 5000 requests per hour :)

@eps1lon
Copy link
Author

eps1lon commented Apr 2, 2020

I'd be open to accept a PR which adds support for disabling the file preview

Could you re-open the issue then? Otherwise I'll likely forget about it.

I'd rather wait and see whether anyone actually hits the 5000 requests per hour

Keep in mind that the rate limit is shared by user not token. Refined github also needs a token to work. So it's not really about 5000 requests to display the tree but whatever you have left after your existing work on the GitHub API.

@brumm brumm reopened this Apr 2, 2020
@brumm
Copy link
Owner

brumm commented Apr 2, 2020

All great points!

@brumm brumm added the wontfix This will not be worked on label Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants