From ba6d86ada2960e1aca8bb1a81321e013027fa5b4 Mon Sep 17 00:00:00 2001 From: Tobias Henkel Date: Tue, 15 Sep 2020 18:33:12 +0200 Subject: Save superfluous api requests in check run reporting When reporting the check run result zuul is currently doing four github api requests: 1. Get repository 2. Get head commit of pr 3. Get check runs of head commit 4. (optional) Create initial version of check run in case start reporting is disabled 5. Update check run that has been created in start reporting The first three requests are basically unneeded if we craft a direct PATCH request and remember the check run id that has been created in the start reporting. This is needed since those run in the critical path of the event processing and can cause large event processing delays in a very busy zuul system. Change-Id: I55df1cc28279bb6923e51686dde8809421486c6a --- zuul/driver/github/githubconnection.py | 169 +++++++++++++-------------------- zuul/driver/github/githubreporter.py | 9 +- 2 files changed, 74 insertions(+), 104 deletions(-) (limited to 'zuul/driver') diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index 1ef5f9a8d..9e5c2bbc4 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -1988,13 +1988,27 @@ class GithubConnection(BaseConnection): pull_request.remove_label(label) log.debug("Removed label %s from %s#%s", label, proj, pr_number) + def _create_or_update_check(self, github, project_name, check_run_id, + **kwargs): + if check_run_id: + # Update an existing check run + url = github.session.build_url( + 'repos', project_name, 'check-runs', str(check_run_id)) + resp = github.session.patch(url, json=kwargs) + else: + # Create a new check run + url = github.session.build_url( + 'repos', project_name, 'check-runs') + resp = github.session.post(url, json=kwargs) + resp.raise_for_status() + return resp.json().get('id') + def updateCheck(self, project, pr_number, sha, status, completed, context, details_url, message, file_comments, external_id, - zuul_event_id=None): + zuul_event_id=None, check_run_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("/") # Always provide an empty list of actions by default actions = [] @@ -2018,7 +2032,7 @@ class GithubConnection(BaseConnection): context ) ) - return errors + return None, errors output = {"title": "Summary", "summary": message} @@ -2041,83 +2055,34 @@ class GithubConnection(BaseConnection): # conclusion, as the status will always be "completed". conclusion = status - # Unless something went wrong during the start reporting of this - # 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() - if c.name == context - ] - except github3.exceptions.GitHubException as exc: - log.error( - "Could not retrieve existing check runs for %s#%s on " - "sha %s: %s", - project, pr_number, sha, str(exc), - ) - - if not check_runs: - log.debug( - "Could not find check run %s for %s#%s on sha %s. " - "Creating a new one", - context, project, pr_number, sha, + if not check_run_id: + log.debug("Could not find check run %s for %s#%s on sha %s. " + "Creating a new one", context, project, pr_number, + sha) + action = 'create' + arguments = dict( + name=context, + head_sha=sha, + conclusion=conclusion, + completed_at=completed_at, + output=output, + details_url=details_url, + external_id=external_id, + actions=actions, ) - - try: - check_run = repository.create_check_run( - name=context, - head_sha=sha, - conclusion=conclusion, - completed_at=completed_at, - output=output, - details_url=details_url, - external_id=external_id, - actions=actions, - ) - except github3.exceptions.GitHubException as exc: - # TODO (felix): Should we retry the check_run creation? - log.error( - "Failed to create check run %s for %s#%s on sha %s: " - "%s", - context, project, pr_number, sha, str(exc) - ) - errors.append( - "Failed to create check run {}: {}".format( - context, str(exc) - ) - ) else: - check_run = check_runs[0] - log.debug( - "Updating existing check run %s for %s#%s on sha %s " - "with status %s", - context, project, pr_number, sha, status, + log.debug("Updating existing check run %s for %s#%s on sha %s " + "with status %s", context, project, pr_number, sha, + status) + action = 'update' + arguments = dict( + conclusion=conclusion, + completed_at=completed_at, + output=output, + details_url=details_url, + external_id=external_id, + actions=actions, ) - - try: - check_run.update( - conclusion=conclusion, - completed_at=completed_at, - output=output, - details_url=details_url, - external_id=external_id, - actions=actions, - ) - except github3.exceptions.GitHubException as exc: - log.error( - "Failed to update check run %s for %s#%s on sha %s: " - "%s", - context, project, pr_number, sha, str(exc), - ) - errors.append( - "Failed to update check run {}: {}".format( - context, str(exc) - ) - ) - else: # Add an abort/dequeue action to running check runs actions.append( @@ -2132,35 +2097,33 @@ class GithubConnection(BaseConnection): } ) - # Report the start of a check run - try: - data = dict( - name=context, - head_sha=sha, - status=status, - output=output, - details_url=details_url, - external_id=external_id, - actions=actions, - ) + action = 'create' + arguments = dict( + name=context, + head_sha=sha, + status=status, + output=output, + details_url=details_url, + external_id=external_id, + actions=actions, + ) - url = github.session.build_url('repos', project, - 'check-runs') - resp = github.session.post(url, json=data) - resp.raise_for_status() + # Create/update the check run + try: + check_run_id = self._create_or_update_check( + github, + project, + check_run_id, + **arguments, + ) - 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), - ) - errors.append( - "Failed to update check run {}: {}".format( - context, str(exc) - ) - ) + except requests.exceptions.HTTPError as exc: + log.error("Failed to %s check run %s for %s#%s on sha %s: %s", + action, context, project, pr_number, sha, str(exc)) + errors.append("Failed to {} check run {}: {}".format( + action, context, str(exc))) - return errors + return check_run_id, errors def _buildAnnotationsFromComments(self, file_comments): annotations = [] diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py index b2487d61e..0925b50d7 100644 --- a/zuul/driver/github/githubreporter.py +++ b/zuul/driver/github/githubreporter.py @@ -245,7 +245,8 @@ class GithubReporter(BaseReporter): } ) - return self.connection.updateCheck( + state = item.dynamic_state[self.connection.connection_name] + check_run_id, errors = self.connection.updateCheck( project, pr_number, sha, @@ -257,8 +258,14 @@ class GithubReporter(BaseReporter): file_comments, external_id, zuul_event_id=item.event, + check_run_id=state.get('check_run_id') ) + if check_run_id: + state['check_run_id'] = check_run_id + + return errors + def setLabels(self, item): log = get_annotated_logger(self.log, item.event) project = item.change.project.name -- cgit v1.2.1