Allow refreshing volatile data in canMerge check

On GitHub we cannot reliably update all information that's needed for
doing a canmerge check using events. Namely completely missing events
on branch protection changes and ambiguous status events that might
match several changes due to its data model to have statuses on the
commit instead the pr. This was no problem in the past since this
information was only used during the enqueue phase which is directly
after the event preprocessing phase.

However with circular dependencies we re-do the can merge check just
before merging again and need to act on recent data. Therefore add an
allow_refresh flag that makes it possible to refresh the volatile
parts of the data we don't get events for. This is only used on GitHub
for now as the other drivers are either correctly updating their
states using events or didn't yet optimize to not do api calls within
the main loop yet (pagure).

Change-Id: I89ff158642fe32c5004ef62c2e25399110564252
This commit is contained in:
Simon Westphahl 2020-12-14 14:54:02 +01:00 committed by Tobias Henkel
parent 5161347efd
commit 29592c9531
7 changed files with 27 additions and 9 deletions

View File

@ -46,7 +46,9 @@ class GerritSource(BaseSource):
def isMerged(self, change, head=None):
return self.connection.isMerged(change, head)
def canMerge(self, change, allow_needs, event=None):
def canMerge(self, change, allow_needs, event=None, allow_refresh=False):
# Gerrit changes have no volatile data that cannot be updated via
# events and thus needs not to act on allow_refresh.
return self.connection.canMerge(change, allow_needs, event=event)
def postConfig(self):

View File

@ -32,7 +32,7 @@ class GitSource(BaseSource):
def isMerged(self, change, head=None):
raise NotImplementedError()
def canMerge(self, change, allow_needs, event=None):
def canMerge(self, change, allow_needs, event=None, allow_refresh=False):
raise NotImplementedError()
def getChange(self, event, refresh=False):

View File

@ -1622,9 +1622,12 @@ class GithubConnection(CachedBranchConnection):
log.debug('Got PR %s#%s', project_name, number)
return (pr, probj)
def canMerge(self, change, allow_needs, event=None):
def canMerge(self, change, allow_needs, event=None, allow_refresh=False):
log = get_annotated_logger(self.log, event)
if allow_refresh:
self._updateCanMergeInfo(change, event)
# If the PR is a draft it cannot be merged.
if change.draft:
log.debug('Change %s can not merge because it is a draft', change)

View File

@ -50,13 +50,15 @@ class GithubSource(BaseSource):
# to perform the merge will ensure this is updated.
return change.is_merged
def canMerge(self, change, allow_needs, event=None):
def canMerge(self, change, allow_needs, event=None, allow_refresh=False):
"""Determine if change can merge."""
if not change.number:
# Not a pull request, considering merged.
return True
return self.connection.canMerge(change, allow_needs, event=event)
return self.connection.canMerge(
change, allow_needs, event=event, allow_refresh=allow_refresh
)
def postConfig(self):
"""Called after configuration has been processed."""

View File

@ -47,7 +47,7 @@ class GitlabSource(BaseSource):
return True
return change.is_merged
def canMerge(self, change, allow_needs, event=None):
def canMerge(self, change, allow_needs, event=None, allow_refresh=True):
"""Determine if change can merge."""
if not change.number:
return True

View File

@ -48,11 +48,13 @@ class PagureSource(BaseSource):
return True
return change.is_merged
def canMerge(self, change, allow_needs, event=None):
def canMerge(self, change, allow_needs, event=None, allow_refresh=False):
"""Determine if change can merge."""
if not change.number:
# Not a pull request, considering merged.
return True
# The pagure connection fetches canmerge on demand in any case so
# allow_refresh is not necessary
return self.connection.canMerge(change, allow_needs, event=event)
def postConfig(self):

View File

@ -41,8 +41,17 @@ class BaseSource(object, metaclass=abc.ABCMeta):
If head is provided the change is checked if it is at head."""
@abc.abstractmethod
def canMerge(self, change, allow_needs, event=None):
"""Determine if change can merge."""
def canMerge(self, change, allow_needs, event=None, allow_refresh=False):
"""Determine if change can merge.
change: The change to check for mergeability
allow_needs: The statuses/votes that are allowed to be missing on a
change, typically the votes the pipeline would set itself
before attempting to merge
event: event information for log annotation
allow_refresh: Allow refreshing of cached volatile data that cannot be
reliably kept up to date using events.
"""
def postConfig(self):
"""Called after configuration has been processed."""