From 59188b984398cbf735346b6eed1e180ad259e843 Mon Sep 17 00:00:00 2001 From: Thierry Carrez Date: Wed, 18 Mar 2020 11:04:25 +0100 Subject: [PATCH] Use /detail in check-release-approval queries Gerrit sometimes returns a partial JSON answer, missing the details that o=DETAILED_LABELS should trigger. This results in false negatives in check-release-approval tests. This cannot be reproduced easily. We put in place a retry but the issue seems to stick on immediate retries. As an experiment, this change switches the API call from /changes/ID with o=DETAILED_LABELS&o=DETAILED_ACCOUNTS to /changes/ID/detail (which includes these two options, amongst others), to see if that would workaround the Gerrit issue. We also remove the retry since it does not improve significantly the situation. Change-Id: I4de49da1b48f7b87879102a0e18e97168e39406b --- roles/check-release-approval/files/check_approval.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/roles/check-release-approval/files/check_approval.py b/roles/check-release-approval/files/check_approval.py index 2997faa25e..e70c600325 100755 --- a/roles/check-release-approval/files/check_approval.py +++ b/roles/check-release-approval/files/check_approval.py @@ -67,17 +67,14 @@ class GerritChange(object): LOG.warning( '\ndata from gerrit is missing required keys:\n\n%s\n', json.dumps(self.raw_data, indent=2)) - LOG.warning("Retrying once...") - self.load_from_gerrit(args.changeid) - LOG.warning("Second try was successful.") + raise self.workspace = args.releases def load_from_gerrit(self, changeid): # Grab changeid details from Gerrit - call = 'changes/%s' % changeid + \ - '?o=CURRENT_REVISION&o=CURRENT_FILES&o=DETAILED_LABELS' + \ - '&o=DETAILED_ACCOUNTS' + call = 'changes/%s/detail' % changeid + \ + '?o=CURRENT_REVISION&o=CURRENT_FILES' raw = requests.get(GERRIT_URL + call) # Gerrit's REST API prepends a JSON-breaker to avoid XSS