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

gen-push-lists only allows 100 files per PR #737

Closed
simoncozens opened this issue Sep 26, 2023 · 3 comments · Fixed by #769
Closed

gen-push-lists only allows 100 files per PR #737

simoncozens opened this issue Sep 26, 2023 · 3 comments · Fixed by #769

Comments

@simoncozens
Copy link
Contributor

Paginating things in a GitHub GraphQL query is a nightmare. Currently the code we have does this:

organization(login: "google") {
projectV2(number: 74) {
id
title
items(first: 100, after: "%s") {
totalCount

and then

gftools/Lib/gftools/push.py

Lines 106 to 112 in cc53fb5

... on PullRequest {
id
files(first: 100) {
nodes {
path
}
}

We correctly loop over multiple pages because we know that there may be more than 100 "items" in the project board, but we assume there are at most 100 files per PR; this isn't a good assumption in something like a big lang update or a catalogue-wide metadata update.

However (and this is where it gets difficult) to make that work you need to say "give me page 2 of the files on page 3 of the items", and you can't use multiple pagination cursors in a GitHub GraphQL query (See also this issue).

So we may need to do this the inefficient way: grab all the items and their associated PRs first, paging through all the pages of items, and then do a query for each PR, paging through all the lists of files.

Urgh.

@m4rc1e
Copy link
Collaborator

m4rc1e commented Sep 26, 2023

Thanks for looking into this Simon. I don't mind seeing what I can do since I wrote the original query.

@davelab6
Copy link
Member

we may need to do this the inefficient way

But if we have totalCount and can gate on if that is over 99, then that inefficient way can happen only as a Plan B, under an else statement..?

@simoncozens
Copy link
Contributor Author

That's really smart. In pseudocode:

if pr.totalCount > 99:
   prs_to_investigate_further.push(pr.id)

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 a pull request may close this issue.

3 participants