refactor: simplify auth flow to avoid Github API calls as much as possible #106
Conversation
…sible Attempts to handle use cases that don't require repository permission lookups first to avoid hitting the Github API. When we do need to look at repository permissions, still attempts to use a global cache of public/private flags for repos so if any user has pulled that repo's info, and it's a public repo we can resolve read related permissions without hitting the API. When we need to load the details for a repository, load the user's full listing of repositories en masse/batch. Whenever we load a repo store the public/private fag in global cache. Improve the test suite to handle many of the typical cases.
|
Thanks, I’ll take a look at this. In the process of moving but will look at it when I can. |
|
I think we are going to test this PR on large scale. Each user (not a Jenkins Admin) who authenticates to our Jenkins with this plugin consumes 2,500 GitHub API calls (❗). I suspect because of the sheer number of GitHub orgs & repos managed by our Jenkins. |
|
@ojacques let's try to build this PR and use it in our test env. |
|
This has been released in github-oauth 0.32 |
| return accessToken; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the Github server used for this token | ||
| * @return githubServer | ||
| */ | ||
| public String getGithubServer() { | ||
| String getGithubServer() { |
There was a problem hiding this comment.
breaks binary compabitility with plugins that already using this method
|
Thanks for pointing out. I’ll update the method to restore the previous call for that method |
| GithubMyself me; | ||
| try { | ||
| me = usersByTokenCache.getIfPresent(token); | ||
| if (me == null) { | ||
| GHMyself ghMyself = getGitHub().getMyself(); | ||
| me = new GithubMyself(ghMyself); | ||
| usersByTokenCache.put(token, me); | ||
| // Also stick into usersByIdCache (to have latest copy) | ||
| String username = ghMyself.getLogin(); | ||
| usersByIdCache.put(username, new GithubUser(ghMyself)); |
There was a problem hiding this comment.
is it possible this code is throwing an exception that is causing https://issues.jenkins-ci.org/browse/JENKINS-56997
It seems like this method is returning UNKNOWN_TOKEN. I'm running into this issue when scanning my organization for a pipeline configuration if that helps.
There was a problem hiding this comment.
Thanks for the note I’ll investigate this
Follow on to #102
I apologize as this is a biggie. The goal here is to try and improve performance for typical users (non-admins) when making heavy use of github org folders/repos.
Attempts to handle use cases that don't require repository permission lookups first to avoid hitting the Github API at all when we can.
When we do need to look at repository permissions, still attempts to use a global cache of public/private flags for repos so if any user has pulled that repo's info and it's a public repo we can resolve read related permissions without hitting the API to grab the user's repos or the specific repo.
When we need to load the details for a repository, load the user's full listing of repositories en masse/batch first, and if not in that listing then load that repo.
Whenever we load any repo, store the public/private flag in the global/all-users cache.