zuul
/
zuul
Code Issues Proposed changes

Remove an unneeded api call when creating check_runs 

During the start reporting the check run is being created in status
in_progress. This currently does two api requests due to the
github3.py object model. The first one is to get the repository
object, the second one to actually create the check run. However we
already have all information available that is required to directly
craft a post request to create that check run.

This is a problem because this start reporting runs within the main
loop of zuul and needs to be as fast as possible. In a very busy zuul
api requests are becoming a bottleneck so we need to remove any
request that is unneeded.

Change-Id: Idd166c61a4d0081c284dee5677b4cc2e48932307
This commit is contained in:
Tobias Henkel 2020-09-15 16:17:01 +02:00
parent 87aa3111a8
commit 418bf6b462
No known key found for this signature in database
GPG Key ID: 03750DEC158E5FA2
2 changed files with 59 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
tests/fakegithub.py
View File

@ -17,6 +17,7 @@ import urllib
from collections import defaultdict
import datetime
import github3.exceptions
import re
import time
@ -87,11 +88,12 @@ class FakeApp:
class FakeCheckRun(object):
def __init__(self, name, details_url, output, status, conclusion,
def __init__(self, id, name, details_url, output, status, conclusion,
completed_at, external_id, actions, app):
if actions is None:
actions = []
self.id = id
self.name = name
self.details_url = details_url
self.output = output
@ -110,6 +112,7 @@ class FakeCheckRun(object):
def as_dict(self):
return {
'id': self.id,
"name": self.name,
"status": self.status,
"output": self.output,
@ -166,9 +169,10 @@ class FakeCommit(object):
# the last status provided for a commit.
self._statuses.insert(0, status)
def set_check_run(self, name, details_url, output, status, conclusion,
def set_check_run(self, id, name, details_url, output, status, conclusion,
completed_at, external_id, actions, app):
check_run = FakeCheckRun(
id,
name,
details_url,
output,
@ -182,6 +186,7 @@ class FakeCommit(object):
# Always insert a check_run to the front of the list to represent the
# last check_run provided for a commit.
self._check_runs.insert(0, check_run)
return check_run
def get_url(self, path, params=None):
if path == 'statuses':
@ -219,6 +224,7 @@ class FakeRepository(object):
self._commits = {}
self.data = data
self.name = name
self.check_run_counter = 0
# Simple dictionary to store permission values per feature (e.g.
# checks, Repository contents, Pull requests, Commit statuses, ...).
@ -303,7 +309,9 @@ class FakeRepository(object):
if commit is None:
commit = FakeCommit(head_sha)
self._commits[head_sha] = commit
self.check_run_counter += 1
commit.set_check_run(
str(self.check_run_counter),
name,
details_url,
output,
@ -568,9 +576,11 @@ class FakeResponse(object):
def raise_for_status(self):
if 400 <= self.status_code < 600:
raise HTTPError(
'{} {}'.format(self.status_code, self.status_message),
response=self)
if isinstance(self.data, str):
text = '{} {}'.format(self.status_code, self.data)
else:
text = '{} {}'.format(self.status_code, self.status_message)
raise HTTPError(text, response=self)
class FakeGithubSession(object):
@ -637,6 +647,39 @@ class FakeGithubSession(object):
}
return FakeResponse(data, 201)
# Handle check run creation
match = re.match(r'.*/repos/(.*)/check-runs$', url)
if match:
org, reponame = match.groups()[0].split('/', 1)
repo = self.client._data.repos.get((org, reponame))
if repo._permissions.get("checks") is False:
# To create a proper github3 exception, we need to mock a
# response object
return FakeResponse(
"Resource not accessible by integration", 403)
head_sha = json.get('head_sha')
commit = repo._commits.get(head_sha, None)
if commit is None:
commit = FakeCommit(head_sha)
repo._commits[head_sha] = commit
repo.check_run_counter += 1
check_run = commit.set_check_run(
str(repo.check_run_counter),
json['name'],
json['details_url'],
json['output'],
json.get('status'),
json.get('conclusion'),
json.get('completed_at'),
json['external_id'],
json['actions'],
json.get('app', 'zuul'),
)
return FakeResponse(check_run.as_dict(), 201)
return FakeResponse(None, 404)
def put(self, url, data=None, headers=None, params=None, json=None):

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

@ -1993,8 +1993,8 @@ class GithubConnection(BaseConnection):
zuul_event_id=None):
log = get_annotated_logger(self.log, zuul_event_id)
github = self.getGithubClient(project, zuul_event_id=zuul_event_id)
self._append_accept_header(github, PREVIEW_CHECKS_ACCEPT)
owner, proj = project.split("/")
repository = github.repository(owner, proj)
# Always provide an empty list of actions by default
actions = []
@ -2045,6 +2045,8 @@ class GithubConnection(BaseConnection):
# change (e.g. the check_run creation failed), there should already
# be a check_run available. If not we will create one.
check_runs = []
repository = github.repository(owner, proj)
try:
check_runs = [
c for c in repository.commit(sha).check_runs()
@ -2132,7 +2134,7 @@ class GithubConnection(BaseConnection):
# Report the start of a check run
try:
check_run = repository.create_check_run(
data = dict(
name=context,
head_sha=sha,
status=status,
@ -2141,8 +2143,13 @@ class GithubConnection(BaseConnection):
external_id=external_id,
actions=actions,
)
except github3.exceptions.GitHubException as exc:
# TODO (felix): Should we retry the check run creation?
url = github.session.build_url('repos', project,
'check-runs')
resp = github.session.post(url, json=data)
resp.raise_for_status()
except requests.exceptions.HTTPError as exc:
log.error(
"Failed to create check run %s for %s#%s on sha %s: %s",
context, project, pr_number, sha, str(exc),