From 6da3e5748f36c909829b92121493500efdd76385 Mon Sep 17 00:00:00 2001 From: Hoa Nguyen Date: Wed, 2 Dec 2020 14:30:47 -0800 Subject: [PATCH] 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 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38236 Tested-by: kokoro Reviewed-by: Jason Lowe-Power Maintainer: Jason Lowe-Power --- util/gerrit-bot/bot.py | 60 +++++++++++++++++++++++++-- util/gerrit-bot/extract_gitcookies.py | 8 ++-- util/gerrit-bot/gerrit.py | 13 +++++- util/gerrit-bot/util.py | 13 ++++-- 4 files changed, 81 insertions(+), 13 deletions(-) diff --git a/util/gerrit-bot/bot.py b/util/gerrit-bot/bot.py index 70be95d25..709279c4e 100755 --- a/util/gerrit-bot/bot.py +++ b/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): diff --git a/util/gerrit-bot/extract_gitcookies.py b/util/gerrit-bot/extract_gitcookies.py index 2ea0468bb..fbe9c80b6 100755 --- a/util/gerrit-bot/extract_gitcookies.py +++ b/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") \ No newline at end of file + output_stream.write(f"{username}\n{password}\n") diff --git a/util/gerrit-bot/gerrit.py b/util/gerrit-bot/gerrit.py index 84f6640d0..7dde34bef 100644 --- a/util/gerrit-bot/gerrit.py +++ b/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) diff --git a/util/gerrit-bot/util.py b/util/gerrit-bot/util.py index bc7045b0c..31b9fcc8a 100644 --- a/util/gerrit-bot/util.py +++ b/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) -- 2.30.2