Fix empty reviewer issue and improve error reporting

diff --git a/plugins/add_reviewers.py b/plugins/add_reviewers.py
index cd574d789d6e87d23f3c614cfa535dff5c1f4f59..dd5c12f53aaeccd02062f4c0c5f80f4b1b545250 100644 (file)
--- a/plugins/add_reviewers.py
+++ b/plugins/add_reviewers.py
@@ -126,8 +126,10 @@ def patchset_created(hook = None, config = None, logger = None, params = None,
         dry_run = False
 
     if reviewers:
+        logger.debug("reviewers: %d %s", len(reviewers), reviewers)
         add_reviewers(reviewers, gerrit, change_info, dry_run, logger)
     if reviewer_groups:
+        logger.debug("reviewer groups: %d %s", len(reviewer_groups), reviewer_groups)
         add_reviewers(reviewer_groups, gerrit, change_info, dry_run, logger)
 
 
@@ -203,9 +205,11 @@ def return_reviewers(rule):
     reviewers = set()
     reviewer_groups = set()
     for reviewer in re.split(r'\s*[,;]\s*', rule.get('reviewer',"")):
-        reviewers.add(reviewer.strip())
+        if reviewer.strip():
+            reviewers.add(reviewer.strip())
     for group in re.split(r'\s*[,;]\s*', rule.get('reviewer_group',"")):
-        reviewer_groups.add(group.strip())
+        if group.strip():
+            reviewer_groups.add(group.strip())
     return list(reviewers), list(reviewer_groups)
 
 
@@ -243,13 +247,19 @@ def get_rules(config, section_name):
 def add_reviewers(reviewers, gerrit, change, dry_run, logger):
     """Adding reviewers for change if dry_run is False"""
     for reviewer in reviewers:
+        if not reviewer:
+            continue
         logger.debug("Adding reviewer %s (dry run: %s)", reviewer, dry_run)
         if not dry_run:
             added = gerrit.add_change_reviewer(change['id'],
                                                 reviewer, confirmed=True)
+            if not added:
+                logger.error("Unknown error adding %s to change %s",
+                                                    reviewer, change['id'])
+                continue
             if 'error' in added:
-                logger.error("Error adding %s to change %s",
-                                            reviewer, change['change_id'])
+                logger.error("Error adding %s to change %s: %s",
+                                reviewer, change['change_id'], added['error'])
             elif 'reviewers' in added and added['reviewers']:
                 for person in added['reviewers']:
                     logger.info("Added reviewer %s to change %s",
@@ -258,5 +268,3 @@ def add_reviewers(reviewers, gerrit, change, dry_run, logger):
                                 person.get('_account_id', None))),
                         change['change_id'])
 
-
-