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:
parent
188fa53915
commit
846b77322a
|
@ -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()
|
||||
|
|
|
@ -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'
|
||||
|
|
Loading…
Reference in New Issue