util: Add reviewers using account ID's when possible

Currently, there are some accounts that cannot be added as a
reviewer due to unknown conflicts associated with the email address.

This commit adds the ability for the bot to use
ReviewerInfo._account_id when possible, and to use email addresses
otherwise.

To reduce the number of queries to the server, a json file will be
created in .data/ to store known account ID's.

Change-Id: I9887bec12d14279e61119a615687a339e3f9c994
Signed-off-by: Hoa Nguyen <hoanguyen@ucdavis.edu>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38236
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>

util/gerrit-bot/bot.py

@@ -30,6 +30,7 @@ from gerrit import GerritResponseParser as Parser
 from gerrit import GerritRestAPI
 from util import add_maintainers_to_change, convert_time_in_seconds
 
+import json
 import time
 
 import sys
@@ -52,6 +53,11 @@ class GerritBotConfig:
         # REST API was made
         default_config.time_tracker_file_path = ".data/prev_query_time"
 
+        # path to the file containing the map each maintainer email address
+        # to the one account id (ie, the "_account_id" field of ReviewerInfo)
+        default_config.maintainer_account_ids_file_path = \
+            ".data/maintainer_ids.json"
+
         # query changes made within 2 days if prev_query_time is not specified
         default_config.default_query_age = "2d"
 
@@ -78,8 +84,9 @@ class GerritBot:
 
         self.account_id = self.__get_bot_account_id()
         self.maintainers = maint.lib.maintainers.Maintainers.from_file(
-                                            self.config.maintainers_file_path)
-
+            self.config.maintainers_file_path)
+        self.maintainer_account_ids = self.__read_maintainer_account_id_file(
+            self.maintainers, self.config.maintainer_account_ids_file_path)
     def __read_auth_file(self, auth_file_path):
         username = ""
         password = ""
@@ -111,6 +118,51 @@ class GerritBot:
             f.write(f"# The above time is the result of calling time.time() "
                     f"in Python.")
 
+    def __read_maintainer_account_id_file(self, maintainers, file_path):
+        account_ids = {}
+        try:
+            with open(file_path, "r") as f:
+                account_ids = json.load(f)
+        except (FileNotFoundError, json.decoder.JSONDecodeError):
+            # create a placeholder file
+            with open(file_path, "w") as f:
+                json.dump(account_ids, f)
+        account_ids = self.__update_maintainer_account_id_file(file_path,
+                                                               maintainers)
+        return account_ids
+
+    def __update_maintainer_account_id_file(self, file_path, maintainers):
+        # get the current map
+        with open(file_path, "r") as f:
+            account_ids = json.load(f)
+        # get maintainer email addresses
+        email_addresses = set()
+        for tag, subsys in maintainers:
+            for maint in subsys.maintainers:
+                email_addresses.add(maint[1])
+        # get account ids of the addresses
+        for email_address in email_addresses:
+            if email_address in account_ids:
+                continue
+            account_id = self.__get_one_account_id(email_address)
+            if account_id:
+                account_ids[email_address] = account_id
+        # write the map to a json file
+        with open(file_path, "w") as f:
+            json.dump(account_ids, f, indent=4)
+        return account_ids
+
+    def __get_one_account_id(self, email_address):
+        query = f"email:{email_address}"
+        response = self.gerrit_api.query_account(query, 1)
+        accounts = Parser.get_json_content(response)
+        if len(accounts) == 0:
+            print(f"warn: unable to obtain the account id of "
+                  f"\"{email_address}\"")
+            print(vars(response))
+            return None
+        return accounts[0]["_account_id"]
+
     def __get_bot_account_id(self):
         account_info = Parser.parse(self.gerrit_api.get_account("self"))
         return account_info._account_id
@@ -147,7 +199,9 @@ class GerritBot:
     def _run(self):
         new_changes = self.__query_new_changes(self.query_age)
         for new_change in new_changes:
-            add_maintainers_to_change(new_change, self.maintainers,
+            add_maintainers_to_change(new_change,
+                                      self.maintainers,
+                                      self.maintainer_account_ids,
                                       self.gerrit_api)
 
     def _post_run(self):

util/gerrit-bot/extract_gitcookies.py

@@ -52,14 +52,14 @@ def parse_gitcookies(input_path):
 
 if __name__ == "__main__":
     parser = argparse.ArgumentParser(
-        description=("Extract username and password from .gitcookies",
-                     "or from the script used to write .gitcookies file")
+        description=("Extract username and password from .gitcookies"
+                     "or from the script used to write .gitcookies file"))
     parser.add_argument("input",
-                        help = ("Path to a .gitcookies file or a file with ",
+                        help = ("Path to a .gitcookies file or a file with "
                                 "a similar format"))
     parser.add_argument("output", help="Path to the output file")
     args = parser.parse_args()
     username_password_dict = parse_gitcookies(args.input)
     with open(args.output, "w") as output_stream:
         for username, password in username_password_dict.items():
-            output_stream.write(f"{username}\n{password}\n")
+            output_stream.write(f"{username}\n{password}\n")

util/gerrit-bot/gerrit.py

@@ -84,6 +84,15 @@ class GerritRestAPI:
         """ get an account detail from an account_id """
         return self._get(f"/accounts/{account_id}")
 
+    # https://gerrit-review.googlesource.com/Documentation/
+    # rest-api-accounts.html#query-account
+    def query_account(self, query, limit = None):
+        """ get accounts based on the query """
+        params = { "q": query }
+        if limit:
+            params["n"] = str(limit)
+        return self._get(f"/accounts/", params)
+
     # --------------- Changes Endpoints ---------------
     # https://gerrit-review.googlesource.com/Documentation/
     # rest-api-changes.html#list-changes
@@ -91,9 +100,9 @@ class GerritRestAPI:
         """ query changes with maximum limit returned queries """
         endpoint = f"/changes/"
         params = { "q": query }
-        if not limit == None:
+        if limit:
             params["n"] = str(limit)
-        if not optional_field == None:
+        if optional_field:
             params["o"] = optional_field
         return self._get(endpoint, params)
 

util/gerrit-bot/util.py

@@ -1,5 +1,3 @@
-#!/usr/bin/env python3
-
 # Copyright (c) 2020 The Regents of the University of California
 # All Rights Reserved.
 #
@@ -56,7 +54,8 @@ def convert_time_in_seconds(delta):
 
 # End of Utility functions
 
-def add_maintainers_to_change(change, maintainers, gerrit_api):
+def add_maintainers_to_change(change, maintainers, maintainers_account_ids,
+                              gerrit_api):
     tags, message = parse_commit_subject(change["subject"])
     change_id = change["id"]
     maintainer_emails = set()
@@ -68,4 +67,10 @@ def add_maintainers_to_change(change, maintainers, gerrit_api):
             print((f"warning: `change-{change_id}` has an unknown tag: "
                    f"`{tag}`"))
     for email in maintainer_emails:
-        gerrit_api.add_reviewer(change_id, email)
+        try:
+            account_id = maintainers_account_ids[email]
+            gerrit_api.add_reviewer(change_id, account_id)
+        except KeyError:
+            # if cannot find the account id of a maintainer
+            # then use the email address
+            gerrit_api.add_reviewer(change_id, email)