commit c7e281618a51165c41a7b462781d585ea22e26ad
author Mike Frysinger <vapier@chromium.org> Tue Feb 25 03:23:04 2020 -0500
committer Commit Bot <commit-bot@chromium.org> Wed Feb 26 00:58:31 2020 +0000
tree d257d96e25e64f01f299f44389e0204364791c38
parent b99f86c876351572d3dfdcd22a875c9b4c473a8e

gerrit: plumb notify support to add/remove reviewers

We have to switch the remove API to use POST so we can include a body.

BUG=None
TEST=adding & removing user to a CL works

Change-Id: I57352ffb44856b0b460a403c00f24efab8eb6f68
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2071322
Reviewed-by: Michael Mortensen <mmortensen@google.com>
Tested-by: Mike Frysinger <vapier@chromium.org>
Commit-Queue: Mike Frysinger <vapier@chromium.org>

lib/gerrit.py

diff --git a/lib/gerrit.py b/lib/gerrit.py
index 7c66b16..4a57e64 100644
--- a/lib/gerrit.py
+++ b/lib/gerrit.py
@@ -112,7 +112,7 @@
       else:
         gob_util.MarkNotPrivate(self.host, change)
 
-  def SetReviewers(self, change, add=(), remove=(), dryrun=False):
+  def SetReviewers(self, change, add=(), remove=(), dryrun=False, notify='ALL'):
     """Modify the list of reviewers on a gerrit change.
 
     Args:
@@ -120,17 +120,18 @@
       add: Sequence of email addresses of reviewers to add.
       remove: Sequence of email addresses of reviewers to remove.
       dryrun: If True, only print what would have been done.
+      notify: A string, parameter controlling gerrit's email generation.
     """
     if add:
       if dryrun:
         logging.info('Would have added %s to "%s"', add, change)
       else:
-        gob_util.AddReviewers(self.host, change, add)
+        gob_util.AddReviewers(self.host, change, add, notify=notify)
     if remove:
       if dryrun:
         logging.info('Would have removed %s to "%s"', remove, change)
       else:
-        gob_util.RemoveReviewers(self.host, change, remove)
+        gob_util.RemoveReviewers(self.host, change, remove, notify=notify)
 
   def GetChangeDetail(self, change_num, verbose=False):
     """Return detailed information about a gerrit change.

lib/gob_util.py

diff --git a/lib/gob_util.py b/lib/gob_util.py
index 2051aa6..3354ab2 100644
--- a/lib/gob_util.py
+++ b/lib/gob_util.py
@@ -668,38 +668,39 @@
   return FetchUrlJson(host, path)
 
 
-def AddReviewers(host, change, add=None):
+def AddReviewers(host, change, add=None, notify=None):
   """Add reviewers to a change."""
   if not add:
     return
   if isinstance(add, six.string_types):
     add = (add,)
+  body = {}
+  if notify:
+    body['notify'] = notify
   path = '%s/reviewers' % _GetChangePath(change)
   for r in add:
-    body = {'reviewer': r}
+    body['reviewer'] = r
     jmsg = FetchUrlJson(host, path, reqtype='POST', body=body, ignore_404=False)
   return jmsg
 
 
-def RemoveReviewers(host, change, remove=None):
+def RemoveReviewers(host, change, remove=None, notify=None):
   """Remove reveiewers from a change."""
   if not remove:
     return
   if isinstance(remove, six.string_types):
     remove = (remove,)
+  body = {}
+  if notify:
+    body['notify'] = notify
   for r in remove:
-    path = '%s/reviewers/%s' % (_GetChangePath(change), r)
+    path = '%s/reviewers/%s/delete' % (_GetChangePath(change), r)
     try:
-      FetchUrl(host, path, reqtype='DELETE', ignore_404=False)
+      FetchUrl(host, path, reqtype='POST', body=body, ignore_204=True)
     except GOBError as e:
       # On success, gerrit returns status 204; anything else is an error.
       if e.http_status != 204:
         raise
-    else:
-      raise GOBError(
-          http_status=200,
-          reason='Unexpectedly received a 200 http status while deleting'
-                 ' reviewer "%s" from change %s' % (r, change))
 
 
 def SetReview(host, change, revision=None, msg=None, labels=None, notify=None):

scripts/gerrit.py

diff --git a/scripts/gerrit.py b/scripts/gerrit.py
index 37c3a41..cfe8321 100644
--- a/scripts/gerrit.py
+++ b/scripts/gerrit.py
@@ -471,7 +471,7 @@
   if add_list or remove_list:
     helper, cl = GetGerrit(opts, cl)
     helper.SetReviewers(cl, add=add_list, remove=remove_list,
-                        dryrun=opts.dryrun)
+                        dryrun=opts.dryrun, notify=opts.notify)
 UserActReviewers.usage = '<CL> <emails...>'