zuul
/
zuul
Code Issues Proposed changes

Make GitHub rate limit logging configurable 

Currently we have fixed rate limit logging in the GitHub driver. This
does a rate limit api call after almost each api call to
GibHub. However if the GitHub Enterprise instance doesn't have a rate
limit configured this logging doesn't make sense and costs an
additional network round trip time for every request. Thus make this
configurable and by default enabled to retain the current behavior.

Change-Id: I39762611ff74e2ab1fb277f8e9c32be0673abfa3
This commit is contained in:
Tobias Henkel 2018-07-05 08:22:01 +02:00
parent adbbf7cdde
commit 9a694815ff
No known key found for this signature in database
GPG Key ID: 03750DEC158E5FA2
3 changed files with 50 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
doc/source/admin/drivers/github.rst
View File

@ -160,6 +160,12 @@ The supported options in ``zuul.conf`` connections are:
Enable or disable ssl verification for GitHub Enterprise. This
is useful for a connection to a test installation.
.. attr:: rate_limit_logging
:default: true
Enable or disable GitHub rate limit logging. If rate limiting is disabled
in GitHub Enterprise this can save some network round trip times.
Trigger Configuration
---------------------
GitHub webhook events can be configured as triggers.

6
releasenotes/notes/rate-limit-logging-cedd9bb7fb5553d9.yaml Normal file
View File

@ -0,0 +1,6 @@
---
features:
- |
GitHub :attr:`<github connection>.rate_limit_logging` now can be configured.
If disabled this can save some network round trip times. This can be
configured per connection.

66
zuul/driver/github/githubconnection.py
View File

@ -443,7 +443,7 @@ class GithubUser(collections.Mapping):
github = self._connection.getGithubClient(self._project)
user = github.user(self._username)
self.log.debug("Initialized data for user %s", self._username)
log_rate_limit(self.log, github)
self._connection.log_rate_limit(self.log, github)
self._data = {
'username': user.login,
'name': user.name,
@ -469,6 +469,13 @@ class GithubConnection(BaseConnection):
self.source = driver.getSource(self)
self.event_queue = queue.Queue()
# Logging of rate limit is optional as this does additional requests
rate_limit_logging = self.connection_config.get(
'rate_limit_logging', 'true')
self._log_rate_limit = True
if rate_limit_logging.lower() == 'false':
self._log_rate_limit = False
if self.server == 'github.com':
self.base_url = GITHUB_BASE_URL
else:
@ -844,7 +851,7 @@ class GithubConnection(BaseConnection):
change.number))
keys.add(key)
self.log.debug("Ran search issues: %s", query)
log_rate_limit(self.log, github)
self.log_rate_limit(self.log, github)
for key in keys:
(proj, num, sha) = key
@ -956,7 +963,7 @@ class GithubConnection(BaseConnection):
branches.extend([x['name'] for x in resp.json()])
log_rate_limit(self.log, github)
self.log_rate_limit(self.log, github)
self._project_branch_cache[project.name] = branches
return self._project_branch_cache[project.name]
@ -995,7 +1002,7 @@ class GithubConnection(BaseConnection):
pr['files'] = [f.filename for f in probj.files()]
pr['labels'] = [l.name for l in issueobj.labels()]
self.log.debug('Got PR %s#%s', project_name, number)
log_rate_limit(self.log, github)
self.log_rate_limit(self.log, github)
return pr
def canMerge(self, change, allow_needs):
@ -1043,7 +1050,7 @@ class GithubConnection(BaseConnection):
pulls.append(pr.as_dict())
self.log.debug('Got PR on project %s for sha %s', project, sha)
log_rate_limit(self.log, github)
self.log_rate_limit(self.log, github)
if len(pulls) > 1:
raise Exception('Multiple pulls found with head sha %s' % sha)
@ -1168,7 +1175,7 @@ class GithubConnection(BaseConnection):
github.pull_request(owner, project, number).reviews()]
self.log.debug('Got reviews for PR %s/%s#%s', owner, project, number)
log_rate_limit(self.log, github)
self.log_rate_limit(self.log, github)
return reviews
def getUser(self, login, project):
@ -1197,7 +1204,7 @@ class GithubConnection(BaseConnection):
perms = repository._get(url, headers=headers)
self.log.debug("Got repo permissions for %s/%s", owner, proj)
log_rate_limit(self.log, github)
self.log_rate_limit(self.log, github)
# no known user, maybe deleted since review?
if perms.status_code == 404:
@ -1213,7 +1220,7 @@ class GithubConnection(BaseConnection):
pull_request = repository.issue(pr_number)
pull_request.create_comment(message)
self.log.debug("Commented on PR %s/%s#%s", owner, proj, pr_number)
log_rate_limit(self.log, github)
self.log_rate_limit(self.log, github)
def mergePull(self, project, pr_number, commit_message='', sha=None):
github = self.getGithubClient(project)
@ -1226,7 +1233,7 @@ class GithubConnection(BaseConnection):
' conflict, original error is %s' % e)
self.log.debug("Merged PR %s/%s#%s", owner, proj, pr_number)
log_rate_limit(self.log, github)
self.log_rate_limit(self.log, github)
if not result:
raise Exception('Pull request was not merged')
@ -1240,7 +1247,7 @@ class GithubConnection(BaseConnection):
statuses = [status.as_dict() for status in commit.statuses()]
self.log.debug("Got commit statuses for sha %s on %s", sha, project)
log_rate_limit(self.log, github)
self.log_rate_limit(self.log, github)
return statuses
def setCommitStatus(self, project, sha, state, url='', description='',
@ -1251,7 +1258,7 @@ class GithubConnection(BaseConnection):
repository.create_status(sha, state, url, description, context)
self.log.debug("Set commit status to %s for sha %s on %s",
state, sha, project)
log_rate_limit(self.log, github)
self.log_rate_limit(self.log, github)
def labelPull(self, project, pr_number, label):
github = self.getGithubClient(project)
@ -1259,7 +1266,7 @@ class GithubConnection(BaseConnection):
pull_request = github.issue(owner, proj, pr_number)
pull_request.add_labels(label)
self.log.debug("Added label %s to %s#%s", label, proj, pr_number)
log_rate_limit(self.log, github)
self.log_rate_limit(self.log, github)
def unlabelPull(self, project, pr_number, label):
github = self.getGithubClient(project)
@ -1267,7 +1274,7 @@ class GithubConnection(BaseConnection):
pull_request = github.issue(owner, proj, pr_number)
pull_request.remove_label(label)
self.log.debug("Removed label %s from %s#%s", label, proj, pr_number)
log_rate_limit(self.log, github)
self.log_rate_limit(self.log, github)
def getPushedFileNames(self, event):
files = set()
@ -1307,6 +1314,24 @@ class GithubConnection(BaseConnection):
self.connection_name)
return True
def log_rate_limit(self, log, github):
if not self._log_rate_limit:
return
try:
rate_limit = github.rate_limit()
remaining = rate_limit['resources']['core']['remaining']
reset = rate_limit['resources']['core']['reset']
except Exception:
return
if github._zuul_user_id:
log.debug('GitHub API rate limit (%s, %s) remaining: %s reset: %s',
github._zuul_project, github._zuul_user_id, remaining,
reset)
else:
log.debug('GitHub API rate limit remaining: %s reset: %s',
remaining, reset)
class GithubWebController(BaseWebController):
@ -1380,21 +1405,6 @@ def _status_as_tuple(status):
return (user, context, state)
def log_rate_limit(log, github):
try:
rate_limit = github.rate_limit()
remaining = rate_limit['resources']['core']['remaining']
reset = rate_limit['resources']['core']['reset']
except Exception:
return
if github._zuul_user_id:
log.debug('GitHub API rate limit (%s, %s) remaining: %s reset: %s',
github._zuul_project, github._zuul_user_id, remaining, reset)
else:
log.debug('GitHub API rate limit remaining: %s reset: %s',
remaining, reset)
def getSchema():
github_connection = v.Any(str, v.Schema(dict))
return github_connection