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

Code review support #110

Merged
merged 3 commits into from
Feb 27, 2021
Merged

Code review support #110

merged 3 commits into from
Feb 27, 2021

Conversation

JulienMasson
Copy link
Contributor

These changes allow to fetch more informations on pull-request.
They are needed to have the code-review support for Github:
magit/forge#266

Max Node Limit Exceeded
Indeed when I add the reviewThreads object in ghub-fetch-repository, I received this error from the server:

if: peculiar error: 
((type . "MAX_NODE_LIMIT_EXCEEDED") 
 (locations ((line . 232) (column . 3))) 
 (message . "By the time this query traverses to the comments connection, it is requesting up to 1,000,000 possible nodes which exceeds the maximum limit of 500,000."))

As workaround I added ghub-fetch-repository-review-threads which is minimal in order to fetch only pullRequests and avoid the error above.

If you don't mind, I would like your opinion on that.
For Gitlab we don't have this issue since we are sending multiple requests to grab all the informations needed (btw much slower).

However for Github, for the moment I don't know how to handle this case ...

@@ -174,12 +175,14 @@ data as the only argument."
:username username
:auth auth
:host host
:headers headers
:forge forge
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel that host and forge should remain to each other, here and elsewhere, because only taken together the fully specify the other side we are communicating with.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes sure I'll do the change in the next version.

@JulienMasson
Copy link
Contributor Author

In the latest version of this PR, I added the function ghub-fetch-review-threads which fetch only diff post from a pull-request.

In that way, we don't hit GitHub GraphQL API limitations:
https://docs.github.com/en/free-pro-team@latest/graphql/overview/resource-limitations

tarsius and others added 3 commits February 27, 2021 20:24
PullRequestReviewThread object is a threaded list of review comments
for a given pull request.

headRefOid and baseRefOid are the respective head and base SHA
reference for a given pull request.

The GitHub GraphQL API has limitations, Individual calls cannot
request more than 500,000 total nodes.  Thus in order to fetch
reviewThreads from a pull-request, we need to fetch these data
in a dedicated function: `ghub-fetch-review-threads'.

Otherwise we will have this kind of error message: "By the time
this query traverses to the comments connection, it is requesting
up to 1,000,000 possible nodes which exceeds the maximum limit of
500,000."

Source:
https://docs.github.com/en/free-pro-team@latest/graphql/overview/resource-limitations
https://docs.github.com/en/free-pro-team@latest/graphql/reference/objects#pullrequestreviewthread

Signed-off-by: Julien Masson <[email protected]>
@tarsius tarsius force-pushed the code-review-support branch from 6b6b9d4 to f26c1f7 Compare February 27, 2021 19:27
@tarsius tarsius merged commit f26c1f7 into magit:master Feb 27, 2021
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.

2 participants