zuul
/
zuul
Code Issues Proposed changes

Fix missing dequeue on synchronized pull request 

When updating a pull request Github can automatically dismiss a
review. In this case Github also sends a pull_request_review
event. This event contains the old head of the pull request which
leads to zuul updating its internal state of the old pull
request. This also updates the updated_at timestamp. Unlike Gerrit
there is no notion of a by time ordering of pull request 'versions' so
we currently rely on the updated_at timestamp to check if a pull
request is newer than another one. So in cases like mentioned above
that fails because the old and new pull request object finally get the
same timestamp which prevents dequeueing of the old pull request.

As the updated_at timestamp is a moving target that is not bound to
the pull request 'version' we can solve that by just not updating the
timestamp if the pull request is updated in the cache. This way the
old pull request object retains its old timestamp and the update check
works.

Change-Id: Ie194ce6571942b5c175709daa3517f68c7c48379
This commit is contained in:
Tobias Henkel 2018-06-22 08:12:41 +02:00
parent 28d3721c01
commit 42cadd93e8
No known key found for this signature in database
GPG Key ID: 03750DEC158E5FA2
2 changed files with 15 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
tests/unit/test_github_driver.py
View File

@ -270,7 +270,13 @@ class TestGithubDriver(ZuulTestCase):
# event update stamp has resolution one second, wait so the latter
# one has newer timestamp
time.sleep(1)
# On a push to a PR Github may emit a pull_request_review event with
# the old head so send that right before the synchronized event.
review_event = A.getReviewAddedEvent('dismissed')
A.addCommit()
self.fake_github.emitEvent(review_event)
self.fake_github.emitEvent(A.getPullRequestSynchronizeEvent())
self.waitUntilSettled()

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

@ -842,8 +842,15 @@ class GithubConnection(BaseConnection):
change.labels = change.pr.get('labels')
# ensure message is at least an empty string
change.message = change.pr.get('body') or ''
change.updated_at = self._ghTimestampToDate(
change.pr.get('updated_at'))
# Note(tobiash): The updated_at timestamp is a moving target that is
# not bound to the pull request 'version' we can solve that by just not
# updating the timestamp if the pull request is updated in the cache.
# This way the old pull request object retains its old timestamp and
# the update check works.
if not change.updated_at:
change.updated_at = self._ghTimestampToDate(
change.pr.get('updated_at'))
change.url = change.pr.get('url')
change.uris = [
'%s/%s/pull/%s' % (self.server, change.project.name,