From f87952ba0647855723acfc5c280256d909bcb986 Mon Sep 17 00:00:00 2001 From: Tobias Henkel Date: Wed, 23 Sep 2020 12:48:42 +0200 Subject: [PATCH] Don't query branch protection on pull request events The pull request based events already have the information if the target branch is protected. Hence we don't need to re-query this information again. Change-Id: I547db8090e13f1103918ce9c5f33002e9a42fcef --- zuul/connection/__init__.py | 11 ++++++++--- zuul/driver/github/githubconnection.py | 22 ++++++++++++++++------ zuul/driver/github/githubmodel.py | 1 + zuul/driver/github/graphql/__init__.py | 2 ++ 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/zuul/connection/__init__.py b/zuul/connection/__init__.py index a5dd641681..7a64d9c065 100644 --- a/zuul/connection/__init__.py +++ b/zuul/connection/__init__.py @@ -247,7 +247,8 @@ class CachedBranchConnection(BaseConnection): self._project_branch_cache_exclude_unprotected.pop(project.name, None) self._project_branch_cache_include_unprotected.pop(project.name, None) - def checkBranchCache(self, project_name: str, event) -> None: + def checkBranchCache(self, project_name: str, event, + protected: bool = None) -> None: """Clear the cache for a project when a branch event is processed This method must be called when a branch event is processed: if the @@ -258,9 +259,13 @@ class CachedBranchConnection(BaseConnection): The project name. :params event: The event, inherit from `zuul.model.TriggerEvent` class. + :params protected: + If defined the caller already knows if the branch is protected + so the query can be skipped. """ - protected = self.isBranchProtected(project_name, event.branch, - zuul_event_id=event) + if protected is None: + protected = self.isBranchProtected(project_name, event.branch, + zuul_event_id=event) if protected is not None: # If the branch appears in the exclude_unprotected cache but diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index ac9073ce69..4f688b3f21 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -435,12 +435,14 @@ class GithubEventProcessor(object): event.zuul_event_id = self.delivery event.timestamp = self.ts project = self.connection.source.getProject(event.project_name) + change = None if event.change_number: - self.connection._getChange(project, - event.change_number, - event.patch_number, - refresh=True, - event=event) + change = self.connection._getChange( + project, + event.change_number, + event.patch_number, + refresh=True, + event=event) self.log.debug("Refreshed change %s,%s", event.change_number, event.patch_number) @@ -448,7 +450,14 @@ class GithubEventProcessor(object): # unprotected branches, we might need to check whether the # branch is now protected. if hasattr(event, "branch") and event.branch: - self.connection.checkBranchCache(project.name, event) + protected = None + if change: + # PR based events already have the information if the + # target branch is protected so take the information + # from there. + protected = change.branch_protected + self.connection.checkBranchCache(project.name, event, + protected=protected) event.project_hostname = self.connection.canonical_hostname self.event = event @@ -1484,6 +1493,7 @@ class GithubConnection(CachedBranchConnection): change.required_contexts = set( canmerge_data['requiredStatusCheckContexts'] ) + change.branch_protected = canmerge_data['protected'] def getGitUrl(self, project: Project): if self.git_ssh_key: diff --git a/zuul/driver/github/githubmodel.py b/zuul/driver/github/githubmodel.py index da06da46fc..9b9295225f 100644 --- a/zuul/driver/github/githubmodel.py +++ b/zuul/driver/github/githubmodel.py @@ -42,6 +42,7 @@ class PullRequest(Change): self.review_decision = None self.required_contexts = set() self.contexts = set() + self.branch_protected = False @property def status(self): diff --git a/zuul/driver/github/graphql/__init__.py b/zuul/driver/github/graphql/__init__.py index d671e9e90c..f0d17f5f01 100644 --- a/zuul/driver/github/graphql/__init__.py +++ b/zuul/driver/github/graphql/__init__.py @@ -107,8 +107,10 @@ class GraphQLClient: 'requiresApprovingReviews') result['requiresCodeOwnerReviews'] = matching_rule.get( 'requiresCodeOwnerReviews') + result['protected'] = True else: result['requiredStatusCheckContexts'] = [] + result['protected'] = False # Check for draft pull_request = nested_get(repository, 'pullRequest')