Merge "Fix merging with submitWholeTopic"

2 files changed, 75 insertions, 69 deletions

diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py
index e3adfefb5..885b428aa 100644
--- a/zuul/driver/gerrit/gerritconnection.py
+++ b/zuul/driver/gerrit/gerritconnection.py
@@ -1205,16 +1205,17 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection):
         self.event_queue.put(event)
 
     def review(self, item, message, submit, labels, checks_api,
-               file_comments, zuul_event_id=None):
+               file_comments, phase1, phase2, zuul_event_id=None):
         if self.session:
             meth = self.review_http
         else:
             meth = self.review_ssh
         return meth(item, message, submit, labels, checks_api,
-                    file_comments, zuul_event_id=zuul_event_id)
+                    file_comments, phase1, phase2,
+                    zuul_event_id=zuul_event_id)
 
     def review_ssh(self, item, message, submit, labels, checks_api,
-                   file_comments, zuul_event_id=None):
+                   file_comments, phase1, phase2, zuul_event_id=None):
         log = get_annotated_logger(self.log, zuul_event_id)
         if checks_api:
             log.error("Zuul is configured to report to the checks API, "
@@ -1223,23 +1224,24 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection):
         change = item.change
         project = change.project.name
         cmd = 'gerrit review --project %s' % project
-        if message:
-            b_len = len(message.encode('utf-8'))
-            if b_len >= GERRIT_HUMAN_MESSAGE_LIMIT:
-                log.info("Message truncated %d > %d" %
-                         (b_len, GERRIT_HUMAN_MESSAGE_LIMIT))
-                message = ("%s... (truncated)" %
-                           message[:GERRIT_HUMAN_MESSAGE_LIMIT - 20])
-            cmd += ' --message %s' % shlex.quote(message)
-        if submit:
+        if phase1:
+            if message:
+                b_len = len(message.encode('utf-8'))
+                if b_len >= GERRIT_HUMAN_MESSAGE_LIMIT:
+                    log.info("Message truncated %d > %d" %
+                             (b_len, GERRIT_HUMAN_MESSAGE_LIMIT))
+                    message = ("%s... (truncated)" %
+                               message[:GERRIT_HUMAN_MESSAGE_LIMIT - 20])
+                cmd += ' --message %s' % shlex.quote(message)
+            for key, val in labels.items():
+                if val is True:
+                    cmd += ' --%s' % key
+                else:
+                    cmd += ' --label %s=%s' % (key, val)
+            if self.version >= (2, 13, 0):
+                cmd += ' --tag autogenerated:zuul:%s' % (item.pipeline.name)
+        if phase2 and submit:
             cmd += ' --submit'
-        for key, val in labels.items():
-            if val is True:
-                cmd += ' --%s' % key
-            else:
-                cmd += ' --label %s=%s' % (key, val)
-        if self.version >= (2, 13, 0):
-            cmd += ' --tag autogenerated:zuul:%s' % (item.pipeline.name)
         changeid = '%s,%s' % (change.number, change.patchset)
         cmd += ' %s' % changeid
         out, err = self._ssh(cmd, zuul_event_id=zuul_event_id)
@@ -1292,8 +1294,13 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection):
                     time.sleep(x * 10)
 
     def review_http(self, item, message, submit, labels,
-                    checks_api, file_comments, zuul_event_id=None):
+                    checks_api, file_comments, phase1, phase2,
+                    zuul_event_id=None):
         change = item.change
+        changeid = "%s~%s~%s" % (
+            urllib.parse.quote(str(change.project), safe=''),
+            urllib.parse.quote(str(change.branch), safe=''),
+            change.id)
         log = get_annotated_logger(self.log, zuul_event_id)
         b_len = len(message.encode('utf-8'))
         if b_len >= GERRIT_HUMAN_MESSAGE_LIMIT:
@@ -1301,53 +1308,51 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection):
                      (b_len, GERRIT_HUMAN_MESSAGE_LIMIT))
             message = ("%s... (truncated)" %
                        message[:GERRIT_HUMAN_MESSAGE_LIMIT - 20])
-        data = dict(message=message,
-                    strict_labels=False)
-        if change.is_current_patchset:
-            if labels:
-                data['labels'] = labels
-            if file_comments:
-                if self.version >= (2, 15, 0):
-                    file_comments = copy.deepcopy(file_comments)
-                    url = item.formatStatusUrl()
-                    for comments in itertools.chain(file_comments.values()):
-                        for comment in comments:
-                            comment['robot_id'] = 'zuul'
-                            comment['robot_run_id'] = \
-                                item.current_build_set.uuid
-                            if url:
-                                comment['url'] = url
-                    data['robot_comments'] = file_comments
-                else:
-                    data['comments'] = file_comments
-        if self.version >= (2, 13, 0):
-            data['tag'] = 'autogenerated:zuul:%s' % (item.pipeline.name)
-        changeid = "%s~%s~%s" % (
-            urllib.parse.quote(str(change.project), safe=''),
-            urllib.parse.quote(str(change.branch), safe=''),
-            change.id)
-        if checks_api:
-            self.report_checks(log, item, changeid, checks_api)
-        if (message or data.get('labels') or data.get('comments')
-            or data.get('robot_comments')):
-            for x in range(1, 4):
-                try:
-                    self.post('changes/%s/revisions/%s/review' %
-                              (changeid, change.commit),
-                              data)
-                    break
-                except HTTPConflictException:
-                    log.exception("Conflict submitting data to gerrit.")
-                    break
-                except HTTPBadRequestException:
-                    log.exception(
-                        "Bad request submitting check data to gerrit.")
-                    break
-                except Exception:
-                    log.exception(
-                        "Error submitting data to gerrit, attempt %s", x)
-                    time.sleep(x * 10)
-        if change.is_current_patchset and submit:
+        data = dict(strict_labels=False)
+        if phase1:
+            data['message'] = message
+            if change.is_current_patchset:
+                if labels:
+                    data['labels'] = labels
+                if file_comments:
+                    if self.version >= (2, 15, 0):
+                        file_comments = copy.deepcopy(file_comments)
+                        url = item.formatStatusUrl()
+                        for comments in itertools.chain(
+                                file_comments.values()):
+                            for comment in comments:
+                                comment['robot_id'] = 'zuul'
+                                comment['robot_run_id'] = \
+                                    item.current_build_set.uuid
+                                if url:
+                                    comment['url'] = url
+                        data['robot_comments'] = file_comments
+                    else:
+                        data['comments'] = file_comments
+            if self.version >= (2, 13, 0):
+                data['tag'] = 'autogenerated:zuul:%s' % (item.pipeline.name)
+            if checks_api:
+                self.report_checks(log, item, changeid, checks_api)
+            if (message or data.get('labels') or data.get('comments')
+                or data.get('robot_comments')):
+                for x in range(1, 4):
+                    try:
+                        self.post('changes/%s/revisions/%s/review' %
+                                  (changeid, change.commit),
+                                  data)
+                        break
+                    except HTTPConflictException:
+                        log.exception("Conflict submitting data to gerrit.")
+                        break
+                    except HTTPBadRequestException:
+                        log.exception(
+                            "Bad request submitting check data to gerrit.")
+                        break
+                    except Exception:
+                        log.exception(
+                            "Error submitting data to gerrit, attempt %s", x)
+                        time.sleep(x * 10)
+        if phase2 and change.is_current_patchset and submit:
             for x in range(1, 4):
                 try:
                     self.post('changes/%s/submit' % (changeid,), {})
diff --git a/zuul/driver/gerrit/gerritreporter.py b/zuul/driver/gerrit/gerritreporter.py
index fbee0e0ec..984b8742a 100644
--- a/zuul/driver/gerrit/gerritreporter.py
+++ b/zuul/driver/gerrit/gerritreporter.py
@@ -35,7 +35,7 @@ class GerritReporter(BaseReporter):
         self._checks_api = action.pop('checks-api', None)
         self._labels = action
 
-    def report(self, item):
+    def report(self, item, phase1=True, phase2=True):
         """Send a message to gerrit."""
         log = get_annotated_logger(self.log, item.event)
 
@@ -70,7 +70,8 @@ class GerritReporter(BaseReporter):
 
         return self.connection.review(item, message, self._submit,
                                       self._labels, self._checks_api,
-                                      comments, zuul_event_id=item.event)
+                                      comments, phase1, phase2,
+                                      zuul_event_id=item.event)
 
     def getSubmitAllowNeeds(self):
         """Get a list of code review labels that are allowed to be