Reduce requests to github when listing project branches

In getProjectBranches in the GitHub driver first gets the repository
object and then iterates over the branches. Creating the repository
object requires a request to GitHub to get the repository
metadata. However we don't need that data and can skip this request if
we do the branch listing request by our own. This will save about half
of the requests required to get the project branches during a
reconfiguration.

Change-Id: Icd96831d467c1562cdec9d4220154c19a2e402a6

tests/fakegithub.py

@@ -124,17 +124,25 @@ class FakeRepository(object):
             self._commits[sha] = commit
         return commit
 
-    def get_url(self, path):
-        entity, request = path.split('/', 1)
+    def get_url(self, path, params=None):
+        if '/' in path:
+            entity, request = path.split('/', 1)
+        else:
+            entity = path
+            request = None
 
         if entity == 'branches':
-            return self.get_url_branches(request)
+            return self.get_url_branches(request, params=params)
         if entity == 'collaborators':
             return self.get_url_collaborators(request)
         else:
             return None
 
-    def get_url_branches(self, path):
+    def get_url_branches(self, path, params=None):
+        if path is None:
+            # request wants a branch list
+            return self.get_url_branch_list(params)
+
         elements = path.split('/')
 
         entity = elements[-1]
@@ -146,6 +154,15 @@ class FakeRepository(object):
             branch = '/'.join(elements)
             return self.get_url_branch(branch)
 
+    def get_url_branch_list(self, params):
+        if params.get('protected') == 1:
+            exclude_unprotected = True
+        else:
+            exclude_unprotected = False
+        branches = [x.as_dict() for x in self.branches(exclude_unprotected)]
+
+        return FakeResponse(branches, 200)
+
     def get_url_branch(self, branch_name):
         for branch in self._branches:
             if branch.name == branch_name:
@@ -279,6 +296,7 @@ class FakeResponse(object):
     def __init__(self, data, status_code=200):
         self.status_code = status_code
         self.data = data
+        self.links = {}
 
     def json(self):
         return self.data
@@ -293,7 +311,7 @@ class FakeGithubSession(object):
         fakepath = '/'.join(args)
         return FAKE_BASE_URL + fakepath
 
-    def get(self, url, headers=None):
+    def get(self, url, headers=None, params=None):
         request = url
         if request.startswith(FAKE_BASE_URL):
             request = request[len(FAKE_BASE_URL):]
@@ -301,19 +319,19 @@ class FakeGithubSession(object):
         entity, request = request.split('/', 1)
 
         if entity == 'repos':
-            return self.get_repo(request)
+            return self.get_repo(request, params=params)
         else:
             # unknown entity to process
             return None
 
-    def get_repo(self, request):
+    def get_repo(self, request, params=None):
         org, project, request = request.split('/', 2)
         project_name = '{}/{}'.format(org, project)
 
         client = FakeGithubClient(self._data)
         repo = client.repo_from_project(project_name)
 
-        return repo.get_url(request)
+        return repo.get_url(request, params=params)
 
 
 class FakeGithubData(object):

zuul/driver/github/githubconnection.py

@@ -927,24 +927,37 @@ class GithubConnection(BaseConnection):
 
     def getProjectBranches(self, project, tenant):
         github = self.getGithubClient(project.name)
-        try:
-            owner, proj = project.name.split('/')
-            repository = github.repository(owner, proj)
-            exclude_unprotected = tenant.getExcludeUnprotectedBranches(project)
-            self._project_branch_cache[project.name] = [
-                branch.name for branch in repository.branches(
-                    protected=exclude_unprotected)]
-            self.log.debug('Got project branches for %s: %s', project.name,
-                           self._project_branch_cache[project.name])
-            log_rate_limit(self.log, github)
-        except github3.exceptions.ForbiddenError as e:
-            self.log.error(str(e), exc_info=True)
-            rate_limit = github.rate_limit()
-            if rate_limit['resources']['core']['remaining'] == 0:
-                self.log.debug("Rate limit exceeded, using stale branch list")
-            else:
-                self.log.error(str(e), exc_info=True)
+        exclude_unprotected = tenant.getExcludeUnprotectedBranches(project)
 
+        url = github.session.build_url('repos', project.name,
+                                       'branches')
+
+        headers = {'Accept': 'application/vnd.github.loki-preview+json'}
+        protected = 1 if exclude_unprotected else 0
+        params = {'per_page': 100, 'protected': protected}
+
+        branches = []
+        while url:
+            resp = github.session.get(
+                url, headers=headers, params=params)
+
+            # check if we need to do further paged calls
+            url = resp.links.get(
+                'next', {}).get('url')
+
+            if resp.status_code == 403:
+                self.log.error(str(resp))
+                rate_limit = github.rate_limit()
+                if rate_limit['resources']['core']['remaining'] == 0:
+                    self.log.warning(
+                        "Rate limit exceeded, using stale branch list")
+                # failed to list branches so use a stale branch list
+                return self._project_branch_cache.get(project.name, [])
+
+            branches.extend([x['name'] for x in resp.json()])
+
+        log_rate_limit(self.log, github)
+        self._project_branch_cache[project.name] = branches
         return self._project_branch_cache[project.name]
 
     def getBranch(self, project_name, branch):