zuul
/
zuul
Code Issues Proposed changes

Only refresh deps if change messages have changed 

We only need to call the refreshDeps method if the Depends-On
list has changed.  That can only happen with a new patchset
(gerrit) or the PR body has changed (github et al).  Add a method
to determine if the PR body has changed so we can reduce the
times where we need to call this method.

Change-Id: Iaa50a274c29347397bc4e10e2c3cefc25e442879
This commit is contained in:
James E. Blair 2021-09-23 15:44:12 -07:00
parent cd4f7539c1
commit 27b677df91
12 changed files with 54 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
tests/base.py
View File

@ -2412,8 +2412,8 @@ class FakeGithubPullRequest(object):
def getPullRequestClosedEvent(self):
return self._getPullRequestEvent('closed')
def getPullRequestEditedEvent(self):
return self._getPullRequestEvent('edited')
def getPullRequestEditedEvent(self, old_body=None):
return self._getPullRequestEvent('edited', old_body)
def addComment(self, message):
self.comments.append(message)
@ -2546,8 +2546,10 @@ class FakeGithubPullRequest(object):
return (name, data)
def editBody(self, body):
old_body = self.body
self.body = body
self._updateTimeStamp()
return self.getPullRequestEditedEvent(old_body=old_body)
def _getRepo(self):
repo_path = os.path.join(self.upstream_root, self.project)
@ -2636,7 +2638,7 @@ class FakeGithubPullRequest(object):
def getPRReference(self):
return '%s/head' % self.number
def _getPullRequestEvent(self, action):
def _getPullRequestEvent(self, action, old_body=None):
name = 'pull_request'
data = {
'action': action,
@ -2668,8 +2670,11 @@ class FakeGithubPullRequest(object):
'installation': {
'id': 123,
},
'changes': {},
'labels': [{'name': l} for l in self.labels]
}
if old_body:
data['changes']['body'] = {'from': old_body}
return (name, data)
def getCommitStatusEvent(self, context, state='success', user='zuul'):

31
tests/unit/test_cross_crd.py
View File

@ -472,8 +472,8 @@ class TestGerritToGithubCRD(ZuulTestCase):
self.assertEqual(B.reported, 1)
# Update A to add A->B (a cycle).
A.editBody('Depends-On: %s\n' % (B.data['url']))
self.fake_github.emitEvent(A.getPullRequestEditedEvent())
self.fake_github.emitEvent(
A.editBody('Depends-On: %s\n' % (B.data['url'])))
self.waitUntilSettled()
# Dependency cycle injected so zuul should have reported again on A
@ -701,9 +701,8 @@ class TestGithubToGerritCRD(ZuulTestCase):
'gerrit/project1', 'master', 'B')
# A Depends-On: B
A.editBody('Depends-On: %s\n' % (B.data['url'],))
self.fake_github.emitEvent(A.getPullRequestEditedEvent())
self.fake_github.emitEvent(
A.editBody('Depends-On: %s\n' % (B.data['url'],)))
self.waitUntilSettled()
self.hold_jobs_in_queue = False
@ -741,12 +740,12 @@ class TestGithubToGerritCRD(ZuulTestCase):
'gerrit/project1', 'master', 'B')
# A Depends-On: B
A.editBody('Depends-On: %s\n' % (B.data['url'],))
event = A.editBody('Depends-On: %s\n' % (B.data['url'],))
tenant = self.scheds.first.sched.abide.tenants.get('tenant-one')
check_pipeline = tenant.layout.pipelines['check']
# Add two dependent changes...
self.fake_github.emitEvent(A.getPullRequestEditedEvent())
self.fake_github.emitEvent(event)
self.waitUntilSettled()
self.assertEqual(len(check_pipeline.getAllItems()), 2)
@ -794,9 +793,8 @@ class TestGithubToGerritCRD(ZuulTestCase):
'gerrit/project1', 'master', 'B')
# A Depends-On: B
A.editBody('Depends-On: %s\n' % (B.data['url'],))
self.fake_github.emitEvent(A.getPullRequestEditedEvent())
self.fake_github.emitEvent(
A.editBody('Depends-On: %s\n' % (B.data['url'],)))
self.waitUntilSettled()
self.scheds.execute(lambda app: app.sched.reconfigure(app.config))
@ -856,9 +854,8 @@ class TestGithubToGerritCRD(ZuulTestCase):
B.subject, C.url)
# A Depends-On: B
A.editBody('Depends-On: %s\n' % (B.data['url'],))
self.fake_github.emitEvent(A.getPullRequestEditedEvent())
self.fake_github.emitEvent(
A.editBody('Depends-On: %s\n' % (B.data['url'],)))
self.waitUntilSettled()
self.assertEqual(self.history[-1].changes, '2,%s 1,1 1,%s' %
(C.head_sha, A.head_sha))
@ -900,14 +897,14 @@ class TestGithubToGerritCRD(ZuulTestCase):
'gerrit/unknown', 'master', 'B')
# A Depends-On: B
A.editBody('Depends-On: %s\n' % (B.data['url'],))
event = A.editBody('Depends-On: %s\n' % (B.data['url'],))
# Make sure zuul has seen an event on B.
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
# Note we wait until settled here as the event processing for
# the next event may not have the updated db yet otherwise.
self.waitUntilSettled()
self.fake_github.emitEvent(A.getPullRequestEditedEvent())
self.fake_github.emitEvent(event)
self.waitUntilSettled()
self.assertFalse(A.is_merged)
@ -927,8 +924,8 @@ class TestGithubToGerritCRD(ZuulTestCase):
# Create B->A
B = self.fake_github.openFakePullRequest('github/project2', 'master',
'B')
B.editBody('Depends-On: %s\n' % (A.data['url'],))
self.fake_github.emitEvent(B.getPullRequestEditedEvent())
self.fake_github.emitEvent(
B.editBody('Depends-On: %s\n' % (A.data['url'],)))
self.waitUntilSettled()
# Dep is there so zuul should have reported on B

3
tests/unit/test_github_crd.py
View File

@ -204,8 +204,7 @@ class TestGithubCrossRepoDeps(ZuulTestCase):
# Update B to point at A1 instead of A
msg = "Depends-On: https://github.com/org/project1/pull/%s" % A1.number
B.editBody(msg)
self.fake_github.emitEvent(B.getPullRequestEditedEvent())
self.fake_github.emitEvent(B.editBody(msg))
self.waitUntilSettled()
# Release

5
tests/unit/test_pagure_driver.py
View File

@ -1019,11 +1019,8 @@ class TestGithubToPagureCRD(ZuulTestCase):
'pagure/project2', 'master', 'B')
# A Depends-On: B
A.editBody('Depends-On: %s\n' % B.url)
self.executor_server.hold_jobs_in_build = True
self.fake_github.emitEvent(A.getPullRequestEditedEvent())
self.fake_github.emitEvent(A.editBody('Depends-On: %s\n' % B.url))
self.waitUntilSettled()
self.assertTrue(self.builds[0].hasChanges(A, B))

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

@ -463,6 +463,8 @@ class GithubEventProcessor(object):
event.label = self.body['label']['name']
elif action == 'edited':
event.action = 'edited'
if 'body' in self.body.get('changes', {}):
event.body_edited = True
else:
return None

6
zuul/driver/github/githubmodel.py
View File

@ -107,6 +107,7 @@ class GithubTriggerEvent(TriggerEvent):
self.check_run = None
self.status = None
self.commits = []
self.body_edited = None
def toDict(self):
d = super().toDict()
@ -118,6 +119,7 @@ class GithubTriggerEvent(TriggerEvent):
d["check_run"] = self.check_run
d["status"] = self.status
d["commits"] = self.commits
d["body_edited"] = self.body_edited
return d
def updateFromDict(self, d):
@ -130,12 +132,16 @@ class GithubTriggerEvent(TriggerEvent):
self.check_run = d["check_run"]
self.status = d["status"]
self.commits = d["commits"]
self.body_edited = d["body_edited"]
def isPatchsetCreated(self):
if self.type == 'pull_request':
return self.action in ['opened', 'changed']
return False
def isMessageChanged(self):
return bool(self.body_edited)
def isChangeAbandoned(self):
if self.type == 'pull_request':
return 'closed' == self.action

2
zuul/driver/gitlab/gitlabconnection.py
View File

@ -124,6 +124,8 @@ class GitlabEventConnector(threading.Thread):
event.patch_number = attrs['last_commit']['id']
event.change_url = self.connection.getMRUrl(event.project_name,
event.change_number)
if attrs['action'] == 'update' and "description" in body["changes"]:
event.merge_request_description_changed = True
if attrs['action'] == 'open':
event.action = 'opened'
elif attrs['action'] == 'merge':

8
zuul/driver/gitlab/gitlabmodel.py
View File

@ -78,6 +78,7 @@ class GitlabTriggerEvent(TriggerEvent):
self.action = None
self.labels = []
self.change_number = None
self.merge_request_description_changed = None
self.tag = None
def toDict(self):
@ -87,6 +88,8 @@ class GitlabTriggerEvent(TriggerEvent):
d["action"] = self.action
d["labels"] = self.labels
d["change_number"] = self.change_number
d["merge_request_description_changed"] = \
self.merge_request_description_changed
d["tag"] = self.tag
return d
@ -97,6 +100,8 @@ class GitlabTriggerEvent(TriggerEvent):
self.action = d["action"]
self.labels = d["labels"]
self.change_number = d["change_number"]
self.merge_request_description_changed = \
d["merge_request_description_changed"]
self.tag = d["tag"]
def _repr(self):
@ -115,6 +120,9 @@ class GitlabTriggerEvent(TriggerEvent):
return self.action in ['opened', 'changed']
return False
def isMessageChanged(self):
return bool(self.merge_request_description_changed)
class GitlabEventFilter(EventFilter):
def __init__(

1
zuul/driver/pagure/pagureconnection.py
View File

@ -242,6 +242,7 @@ class PagureEventConnector(threading.Thread):
""" Handles pull request initial comment change """
event, _ = self._event_base(body)
event.action = 'changed'
event.initial_comment_changed = True
return event
def _event_pull_request_tags_changed(self, body):

6
zuul/driver/pagure/paguremodel.py
View File

@ -87,6 +87,7 @@ class PagureTriggerEvent(TriggerEvent):
self.trigger_name = 'pagure'
self.title = None
self.action = None
self.initial_comment_changed = None
self.status = None
self.tags = []
self.tag = None
@ -96,6 +97,7 @@ class PagureTriggerEvent(TriggerEvent):
d["trigger_name"] = self.trigger_name
d["title"] = self.title
d["action"] = self.action
d["initial_comment_changed"] = self.initial_comment_changed
d["status"] = self.status
d["tags"] = list(self.tags)
d["tag"] = self.tag
@ -106,6 +108,7 @@ class PagureTriggerEvent(TriggerEvent):
self.trigger_name = d.get("trigger_name", "pagure")
self.title = d.get("title")
self.action = d.get("action")
self.initial_comment_changed = d.get("initial_comment_changed")
self.status = d.get("status")
self.tags = d.get("tags", [])
self.tag = d.get("tag")
@ -128,6 +131,9 @@ class PagureTriggerEvent(TriggerEvent):
return self.action in ['opened', 'changed']
return False
def isMessageChanged(self):
return bool(self.initial_comment_changed)
class PagureEventFilter(EventFilter):
def __init__(self, connection_name, trigger, types=[], refs=[],

3
zuul/model.py
View File

@ -4422,6 +4422,9 @@ class TriggerEvent(AbstractEvent):
def isPatchsetCreated(self):
return False
def isMessageChanged(self):
return False
def isChangeAbandoned(self):
return False

7
zuul/scheduler.py
View File

@ -1750,11 +1750,8 @@ class Scheduler(threading.Thread):
# Let the pipeline update any dependencies that may need
# refreshing if this change has updated.
# TODO: We only really need to run this if we have a new
# patchest (isPatchsetCreated()) or the depends-on list in the
# PR body has changed (but we don't have a way to test that yet).
pipeline.manager.refreshDeps(change, event)
if event.isPatchsetCreated() or event.isMessageChanged():
pipeline.manager.refreshDeps(change, event)
if event.isPatchsetCreated():
pipeline.manager.removeOldVersionsOfChange(change, event)