Hoa Nguyen has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/38236 )

Change subject: util: Add reviewers using account ID's when possible
......................................................................

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 <[email protected]>
---
M util/gerrit-bot/bot.py
M util/gerrit-bot/gerrit.py
M util/gerrit-bot/util.py
3 files changed, 77 insertions(+), 10 deletions(-)



diff --git a/util/gerrit-bot/bot.py b/util/gerrit-bot/bot.py
index 70be95d..709279c 100755
--- a/util/gerrit-bot/bot.py
+++ b/util/gerrit-bot/bot.py
@@ -30,6 +30,7 @@
 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 @@
         # 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 @@

         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 @@
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 @@
     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):
diff --git a/util/gerrit-bot/gerrit.py b/util/gerrit-bot/gerrit.py
index 84f6640..7dde34b 100644
--- a/util/gerrit-bot/gerrit.py
+++ b/util/gerrit-bot/gerrit.py
@@ -84,6 +84,15 @@
         """ 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 @@
         """ 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)

diff --git a/util/gerrit-bot/util.py b/util/gerrit-bot/util.py
index bc7045b..e04c9eb 100644
--- a/util/gerrit-bot/util.py
+++ b/util/gerrit-bot/util.py
@@ -1,6 +1,3 @@
-#!/usr/bin/env python3
-
-# Copyright (c) 2020 The Regents of the University of California
 # All Rights Reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -56,7 +53,8 @@

 # 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 +66,10 @@
             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)

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38236
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I9887bec12d14279e61119a615687a339e3f9c994
Gerrit-Change-Number: 38236
Gerrit-PatchSet: 1
Gerrit-Owner: Hoa Nguyen <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to