GitHub Pagination #1804
Merged
GitHub Pagination #1804
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
russjones
force-pushed
the
rjones/web-linking
branch
from
8a9a318 to
5265384
Compare
klizhentas
reviewed
Mar 21, 2018
lib/auth/github.go
Outdated
| @@ -274,6 +274,8 @@ func populateGithubClaims(client githubAPIClientI) (*services.GithubClaims, erro | |||
| if err != nil { | |||
| return nil, trace.Wrap(err, "failed to query Github user teams") | |||
| } | |||
| log.Debugf("Retrieved %v teams for GitHub user %v", len(teams), user.Login) | |||
There was a problem hiding this comment.
missing dot. Also you can use structured logging to list teams:
log.WithFields(log.Fields{"teams:" teams, "user": user.Login})There was a problem hiding this comment.
The full list of teams is printed a few lines below. I added this line as a quick sanity check when we are debugging things for customers. For example maybe pagination breaks at some point in the future, this is a good diagnostic line to tell at a glance something is wrong.
lib/auth/github.go
Outdated
|
|
||
| // If the response returned a next page link, continue following the next | ||
| // page links until all teams have been retrieved. | ||
| for nextPage != "" { |
There was a problem hiding this comment.
can you limit the number of pages to some reasonably high number? E.g. if github API goes crazy and will start returning circular link, this logic will hang forever.
russjones
force-pushed
the
rjones/web-linking
branch
from
5265384 to
1a62aca
Compare
r0mant
approved these changes
Mar 21, 2018
russjones
force-pushed
the
rjones/web-linking
branch
from
1a62aca to
e06521d
Compare
klizhentas
approved these changes
Mar 21, 2018
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Teleport was only returning the first page of results when querying the
GET /user/teamsendpoint. This results in aUnable to process callback from Githuberror for users with more than 30 teams.Implementation
When processing the response to a GET request, extract any pagination links from the response header. Use the next link to paginate over all teams and return the full result set.
Related Issues
Fixes #1734