Skip to content

Avoid over-fetching from the API in pr commands #2849

@mislav

Description

@mislav

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:

cli/api/queries_pr.go

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    coreThis issue is not accepting PRs from outside contributorsenhancementa request to improve CLI

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions