-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Closed
Labels
coreThis issue is not accepting PRs from outside contributorsThis issue is not accepting PRs from outside contributorsenhancementa request to improve CLIa request to improve CLI
Description
gh pr view 123 --web is now slow because of the giant list of PR-related fields that we query from the server. However, the --web mode only ever needs one field: the URL.
This was just an example, but different gh pr commands need to fetch a different set of fields, so we shouldn't just reuse one giant query for all of them:
Lines 533 to 604 in d0a4639
| query PullRequestByNumber($owner: String!, $repo: String!, $pr_number: Int!) { | |
| repository(owner: $owner, name: $repo) { | |
| pullRequest(number: $pr_number) { | |
| id | |
| url | |
| number | |
| title | |
| state | |
| closed | |
| body | |
| mergeable | |
| author { | |
| login | |
| } | |
| ` + statusesFragment + ` | |
| baseRefName | |
| headRefName | |
| headRepositoryOwner { | |
| login | |
| } | |
| headRepository { | |
| name | |
| } | |
| isCrossRepository | |
| isDraft | |
| maintainerCanModify | |
| reviewRequests(first: 100) { | |
| nodes { | |
| requestedReviewer { | |
| __typename | |
| ...on User { | |
| login | |
| } | |
| ...on Team { | |
| name | |
| } | |
| } | |
| } | |
| totalCount | |
| } | |
| assignees(first: 100) { | |
| nodes { | |
| login | |
| } | |
| totalCount | |
| } | |
| labels(first: 100) { | |
| nodes { | |
| name | |
| } | |
| totalCount | |
| } | |
| projectCards(first: 100) { | |
| nodes { | |
| project { | |
| name | |
| } | |
| column { | |
| name | |
| } | |
| } | |
| totalCount | |
| } | |
| milestone{ | |
| title | |
| } | |
| ` + commentsFragment() + ` | |
| ` + reactionGroupsFragment() + ` | |
| } | |
| } | |
| }` | |
Potential solutions:
- Have the caller of the API query specify the struct to deserialize in. The fields of the struct itself would dictate which GraphQL fields get requested. This way, the caller ensures that every queried field will also be consumed. We spiked a potential approach such as that a while ago, but it was messy and was in a now-deleted branch referenced in Consistent PR lookup interface #1020
- Allow composable queries in which the caller opts in to receive specific sets of information related to a PR. We've spiked an approach like this in the past, but it was based on string GraphQL queries and thus had too many limitations: Add mechanism for combining several GraphQL queries into one #190
- Maintain a different query function in Go for each different command. This way, expanding a set of fields for one command does not affect another.
Whichever approach we take, I would suggest that we look beyond our rudimentary api.GraphQL() method or the shurcooL/graphql client. I think we need a more sophisticated querying approach to address the problems above but also to enable us to safely perform bulk lookups.
Ref. #757
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
coreThis issue is not accepting PRs from outside contributorsThis issue is not accepting PRs from outside contributorsenhancementa request to improve CLIa request to improve CLI