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

(projects) Increase Page limit #232

Closed
wants to merge 3 commits into from

Conversation

timlevett
Copy link
Member

@timlevett timlevett commented Jun 23, 2023

Increase page limit to 12 to match GitHub project issue limits

Reference documentation; https://docs.github.com/en/issues/planning-and-tracking-with-projects/managing-items-in-your-project/adding-items-to-your-project

Increase page limit to 10 to match GitHub project issue limits
@timlevett timlevett requested review from a team, zoltanbedi and mdvictor and removed request for a team June 23, 2023 16:28
can have up to 1200 turns out
@timlevett timlevett requested review from scottlepp and removed request for zoltanbedi June 23, 2023 16:39
@scottlepp scottlepp removed their request for review June 30, 2023 12:02
@mdvictor
Copy link
Contributor

I understand the need to match GitHubs limit, but this will potentially cause other problems because of their rate limit. We have situations where running graphql project queries through the ds will halt because we reach the limits after only a handful of queries are ran. This is, from my understanding, caused by the fact that for a big org like grafana, the api needs to scan through potentially thousands of items before returning the first 100 (as we have set it in code, for each page). And then we would repeat that call 11 more times with this change, since we continue appending items to the dataframe before returning the data to the frontend. So the cost raises quite a lot and I'm not exactly sure at this moment why we have a limit for 100 items per query call. Here is a doc explaining resource limits better for their graphql api.

I think that we could merge this change but should not release the plugin until we have a better understanding of why we query projects the way we do and look for a fix there also.

@timlevett
Copy link
Member Author

@mdvictor i agree with your assessment. I did a bit of digging into how they calculate rate limits in their GraphQL API as you suggested and looked at the project code. I could see why we are hitting the 5000 points per hour.

Basically any call to projects we get project items we grab 100 items, and 100 fields, for each item we grab 100 field values, 10 assignees, and 10 reviewers. If you add that up its 12,100 points per call (max). We then page over these right now 2 pages max (24,100 max). I looked at increasing to 12 pages to match the limits you can have on issues in a project board but we may want to reduce down the points used for each call before doing that. I was thinking it might be good to reduce the field values, or maybe select specific ones?

@mdvictor
Copy link
Contributor

That could be a good idea. I've yet to look into the exact costs of our requests for projects, if I recall correctly reading through the docs there is an endpoint that returns the exact cost of a request so we'll use that to further understand. But we definitely need to address the issue here. Thanks for taking the time to look into this! I would say let's leave this PR as is for now while we look into the projects costs in the following period and fixing this.

@zoltanbedi zoltanbedi requested review from a team, gabor and gwdawson and removed request for a team August 21, 2023 12:27
@gabor
Copy link
Contributor

gabor commented May 3, 2024

we cannot merge the PR currently because of the worries about hitting rate limits. after a discussion with @timlevett , we created a new github-issue describing the situation and possible ways to move forward ( #302 ). we'll close this PR for now.

@gabor gabor closed this May 3, 2024
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.

3 participants