gerrit: use change: when querying changes

when querying Gerrit without a search operator, it would attempt to match based on:
* numerical id
* Change-Id
* commit SHA1

On a Gerrit installation big enough (Wikimedia has half a million
changes) there will be collisions between the numerical id Zuul queries
and SHA1 or Change-Id.

A real world example I have encountered is querying for '433653' which
yields:

 change I02d72045ca288ff518d59cc9675a9e587784722f
   project: wikidata/query/gui
   number: 431713   # <--- not expected
   patchset:
     number: 1
     sha1: 433653c0bb99fd520a2a161ac4e704da9bcff8bf
           ^^^^^^

 change Iedd0f3217c185382eb0ca35c59a4cd2f65de43c8
   project: mediawiki/extensions/MediaWikiFarm
   number: 433653

Zuul proceed with the first, and innapropriate, change. Turns out that
change did not merge on wikidata/query/gui repository but it is the one
zuul-merger attempted to merge. That then caused the actual change to be
rejected by zuul-merger even when the latest patchset was on tip of
branch.

I have also noticed a fair share of changes that kept being updated for
no good reason, possibly due to the wrong change being picked up for
refresh.

In GerritConnection:

Introduce queryChange() which always prepend the 'change:' search
operator, ensuring Gerrit does not yield a change that matches on
Change-Id or patchset SHA1.

Adjust the two calls query() calls in _updateChange() and isMerged() to
use the new method.

Downstream bug: https://phabricator.wikimedia.org/T198968

Change-Id: I08ee908a340f7d3c923b7054ec79cb921bf6571b
This commit is contained in:
Antoine Musso 2018-11-27 15:05:54 +01:00
parent 188fa53915
commit 846b77322a
2 changed files with 10 additions and 2 deletions

View File

@ -728,6 +728,11 @@ class FakeGerritConnection(gerritconnection.GerritConnection):
change.setReported()
def query(self, number):
if type(number) == int:
return self.queryChange(number)
raise Exception("Could not query %s %s" % (type(number, number)))
def queryChange(self, number):
change = self.changes.get(int(number))
if change:
return change.query()

View File

@ -541,7 +541,7 @@ class GerritConnection(BaseConnection):
return change
self.log.info("Updating %s" % (change,))
data = self.query(change.number)
data = self.queryChange(change.number)
change._data = data
if change.patchset is None:
@ -673,7 +673,7 @@ class GerritConnection(BaseConnection):
# means it's merged.
return True
data = self.query(change.number)
data = self.queryChange(change.number)
change._data = data
change.is_merged = self._isMerged(change)
if change.is_merged:
@ -895,6 +895,9 @@ class GerritConnection(BaseConnection):
(pprint.pformat(data)))
return data
def queryChange(self, number):
return self.query('change:%s' % number)
def simpleQuery(self, query):
def _query_chunk(query):
args = '--commit-message --current-patch-set'