From fa1e4c075e76ef5777c4d568eb14db87991cb25d Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Wed, 19 Jun 2024 16:42:07 -0400 Subject: [PATCH 01/18] Initial rule --- bugbot/rules/inactive_patch_author.py | 89 +++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 bugbot/rules/inactive_patch_author.py diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py new file mode 100644 index 000000000..082e1621e --- /dev/null +++ b/bugbot/rules/inactive_patch_author.py @@ -0,0 +1,89 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this file, +# You can obtain one at http://mozilla.org/MPL/2.0/. + +from typing import Dict + +from libmozdata.connection import Connection +from libmozdata.phabricator import PhabricatorAPI +from tenacity import retry, stop_after_attempt, wait_exponential + +from bugbot import utils +from bugbot.bzcleaner import BzCleaner +from bugbot.user_activity import PHAB_CHUNK_SIZE, UserActivity, UserStatus + + +class InactivePatchAuthor(BzCleaner): + """Bugs with patches from inactive authors""" + + def __init__(self): + """Constructor""" + super(InactivePatchAuthor, self).__init__() + self.phab = PhabricatorAPI(utils.get_login_info()["phab_api_key"]) + self.user_activity = UserActivity(phab=self.phab) + + def description(self): + return "Bugs with patches from inactive authors" + + def columns(self): + return ["id", "summary", "author"] + + def get_bugs(self, date="today", bug_ids=[], chunk_size=None): + bugs = super().get_bugs(date, bug_ids, chunk_size) + rev_ids = {rev_id for bug in bugs.values() for rev_id in bug["rev_ids"]} + revisions = self._get_revisions_with_inactive_authors(list(rev_ids)) + + for bugid, bug in list(bugs.items()): + inactive_authors = [ + revisions[rev_id] for rev_id in bug["rev_ids"] if rev_id in revisions + ] + if inactive_authors: + bug["authors"] = inactive_authors + self._unassign_inactive_authors(bugid, inactive_authors) + else: + del bugs[bugid] + + self.query_url = utils.get_bz_search_url({"bug_id": ",".join(bugs.keys())}) + return bugs + + def _unassign_inactive_authors(self, bugid: str, inactive_authors: list) -> None: + comment = "The author of this patch has been inactive. The patch has been unassigned for others to take over." + for author in inactive_authors: + self.phab.request( + "differential.revision.edit", + { + "transactions": [{"type": "authorPHID", "value": None}], + "objectIdentifier": author["rev_id"], + }, + ) + self.autofix_changes[bugid] = {"comment": {"body": comment}} + + def _get_revisions_with_inactive_authors(self, rev_ids: list) -> Dict[int, dict]: + revisions = [] + for _rev_ids in Connection.chunks(rev_ids, PHAB_CHUNK_SIZE): + for revision in self._fetch_revisions(_rev_ids): + revisions.append(revision) + + user_phids = {revision["fields"]["authorPHID"] for revision in revisions} + users = self.user_activity.get_phab_users_with_status(list(user_phids)) + + result = {} + for revision in revisions: + author_info = users[revision["fields"]["authorPHID"]] + if author_info["status"] != UserStatus.ACTIVE: + result[revision["id"]] = { + "rev_id": revision["id"], + "title": revision["fields"]["title"], + "author": author_info, + } + return result + + @retry(wait=wait_exponential(min=4), stop=stop_after_attempt(5)) + def _fetch_revisions(self, ids: list): + return self.phab.request( + "differential.revision.search", constraints={"ids": ids} + )["data"] + + +if __name__ == "__main__": + InactivePatchAuthor().run() From c2a1ce721153d226bec7d0ce1466350071fc37d7 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Fri, 21 Jun 2024 12:13:06 -0400 Subject: [PATCH 02/18] Simplified code --- bugbot/rules/inactive_patch_author.py | 130 +++++++++++++++++--------- 1 file changed, 87 insertions(+), 43 deletions(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index 082e1621e..4222fc03d 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -2,7 +2,8 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this file, # You can obtain one at http://mozilla.org/MPL/2.0/. -from typing import Dict +import re +from typing import Dict, List from libmozdata.connection import Connection from libmozdata.phabricator import PhabricatorAPI @@ -12,78 +13,121 @@ from bugbot.bzcleaner import BzCleaner from bugbot.user_activity import PHAB_CHUNK_SIZE, UserActivity, UserStatus +PHAB_FILE_NAME_PAT = re.compile(r"phabricator-D([0-9]+)-url\.txt") -class InactivePatchAuthor(BzCleaner): - """Bugs with patches from inactive authors""" + +class InactivePatchAuthors(BzCleaner): + """Bugs with patches authored by inactive patch authors""" def __init__(self): - """Constructor""" - super(InactivePatchAuthor, self).__init__() + super(InactivePatchAuthors, self).__init__() self.phab = PhabricatorAPI(utils.get_login_info()["phab_api_key"]) - self.user_activity = UserActivity(phab=self.phab) + self.user_activity = UserActivity(include_fields=["nick"], phab=self.phab) def description(self): - return "Bugs with patches from inactive authors" + return "Bugs with inactive patch authors" def columns(self): - return ["id", "summary", "author"] + return ["id", "summary", "authors"] def get_bugs(self, date="today", bug_ids=[], chunk_size=None): bugs = super().get_bugs(date, bug_ids, chunk_size) rev_ids = {rev_id for bug in bugs.values() for rev_id in bug["rev_ids"]} - revisions = self._get_revisions_with_inactive_authors(list(rev_ids)) + inactive_authors = self._get_inactive_patch_authors(list(rev_ids)) for bugid, bug in list(bugs.items()): - inactive_authors = [ - revisions[rev_id] for rev_id in bug["rev_ids"] if rev_id in revisions + inactive_patches = [ + inactive_authors[rev_id] + for rev_id in bug["rev_ids"] + if rev_id in inactive_authors ] - if inactive_authors: - bug["authors"] = inactive_authors - self._unassign_inactive_authors(bugid, inactive_authors) + + if inactive_patches: + bug["authors"] = inactive_patches + print(f"Bug {bugid} has inactive patch authors: {inactive_patches}") else: del bugs[bugid] - self.query_url = utils.get_bz_search_url({"bug_id": ",".join(bugs.keys())}) return bugs - def _unassign_inactive_authors(self, bugid: str, inactive_authors: list) -> None: - comment = "The author of this patch has been inactive. The patch has been unassigned for others to take over." - for author in inactive_authors: - self.phab.request( - "differential.revision.edit", - { - "transactions": [{"type": "authorPHID", "value": None}], - "objectIdentifier": author["rev_id"], - }, - ) - self.autofix_changes[bugid] = {"comment": {"body": comment}} - - def _get_revisions_with_inactive_authors(self, rev_ids: list) -> Dict[int, dict]: - revisions = [] + def _get_inactive_patch_authors(self, rev_ids: list) -> Dict[int, dict]: + revisions: List[dict] = [] + for _rev_ids in Connection.chunks(rev_ids, PHAB_CHUNK_SIZE): for revision in self._fetch_revisions(_rev_ids): - revisions.append(revision) - - user_phids = {revision["fields"]["authorPHID"] for revision in revisions} - users = self.user_activity.get_phab_users_with_status(list(user_phids)) - - result = {} + author_phid = revision["fields"]["authorPHID"] + created_at = revision["fields"]["dateCreated"] + revisions.append( + { + "rev_id": revision["id"], + "author_phid": author_phid, + "created_at": created_at, + } + ) + + user_phids = {rev["author_phid"] for rev in revisions} + users = self.user_activity.get_phab_users_with_status( + list(user_phids), keep_active=False + ) + + result: Dict[int, dict] = {} for revision in revisions: - author_info = users[revision["fields"]["authorPHID"]] + author_info = users[revision["author_phid"]] if author_info["status"] != UserStatus.ACTIVE: - result[revision["id"]] = { - "rev_id": revision["id"], - "title": revision["fields"]["title"], - "author": author_info, + result[revision["rev_id"]] = { + "author": author_info["phab_username"], + "status": author_info["status"], + "last_active": author_info.get("last_seen"), } + return result - @retry(wait=wait_exponential(min=4), stop=stop_after_attempt(5)) + @retry( + wait=wait_exponential(min=4), + stop=stop_after_attempt(5), + ) def _fetch_revisions(self, ids: list): return self.phab.request( - "differential.revision.search", constraints={"ids": ids} + "differential.revision.search", + constraints={"ids": ids}, )["data"] + def handle_bug(self, bug, data): + rev_ids = [ + int(attachment["file_name"][13:-8]) + for attachment in bug["attachments"] + if attachment["content_type"] == "text/x-phabricator-request" + and PHAB_FILE_NAME_PAT.match(attachment["file_name"]) + and not attachment["is_obsolete"] + ] + + if not rev_ids: + return + + bugid = str(bug["id"]) + data[bugid] = { + "rev_ids": rev_ids, + } + return bug + + def get_bz_params(self, date): + fields = [ + "comments.raw_text", + "comments.creator", + "attachments.file_name", + "attachments.content_type", + "attachments.is_obsolete", + ] + params = { + "include_fields": fields, + "resolution": "---", + "f1": "attachments.ispatch", + "o1": "equals", + "v1": "1", + } + + return params + if __name__ == "__main__": - InactivePatchAuthor().run() + InactivePatchAuthors().run() From 216fb909e4c31139cdbcbbb46cb470ccbd47600d Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Fri, 21 Jun 2024 14:38:05 -0400 Subject: [PATCH 03/18] Added check for inactive patch authors --- bugbot/rules/inactive_patch_author.py | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index 4222fc03d..ac7a8d8a7 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -37,14 +37,14 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): for bugid, bug in list(bugs.items()): inactive_patches = [ - inactive_authors[rev_id] + {"rev_id": rev_id, "author": inactive_authors[rev_id]} for rev_id in bug["rev_ids"] if rev_id in inactive_authors ] if inactive_patches: - bug["authors"] = inactive_patches - print(f"Bug {bugid} has inactive patch authors: {inactive_patches}") + bug["inactive_patches"] = inactive_patches + print(f"Bug {bugid} has inactive patches: {inactive_patches}") else: del bugs[bugid] @@ -57,6 +57,8 @@ def _get_inactive_patch_authors(self, rev_ids: list) -> Dict[int, dict]: for revision in self._fetch_revisions(_rev_ids): author_phid = revision["fields"]["authorPHID"] created_at = revision["fields"]["dateCreated"] + if author_phid == "PHID-USER-eltrc7x5oplwzfguutrb": + continue revisions.append( { "rev_id": revision["id"], @@ -65,19 +67,28 @@ def _get_inactive_patch_authors(self, rev_ids: list) -> Dict[int, dict]: } ) - user_phids = {rev["author_phid"] for rev in revisions} + user_phids = set() + + for revision in revisions: + user_phids.add(revision["author_phid"]) + users = self.user_activity.get_phab_users_with_status( list(user_phids), keep_active=False ) result: Dict[int, dict] = {} for revision in revisions: - author_info = users[revision["author_phid"]] - if author_info["status"] != UserStatus.ACTIVE: + author_phid = revision["author_phid"] + + if author_phid not in users: + continue + + author_info = users[author_phid] + if author_info["status"] == UserStatus.INACTIVE: result[revision["rev_id"]] = { - "author": author_info["phab_username"], + "name": author_info["name"], "status": author_info["status"], - "last_active": author_info.get("last_seen"), + "last_active": author_info.get("last_seen_date"), } return result From 5b933bdafedf0368d7c350e320036d9b826aed9d Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Mon, 24 Jun 2024 13:15:22 -0400 Subject: [PATCH 04/18] Added email template --- bugbot/rules/inactive_patch_author.py | 2 +- templates/inactive_patch_author.html | 31 +++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 templates/inactive_patch_author.html diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index ac7a8d8a7..c88bb0a54 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -28,7 +28,7 @@ def description(self): return "Bugs with inactive patch authors" def columns(self): - return ["id", "summary", "authors"] + return ["id", "summary", "revisions"] def get_bugs(self, date="today", bug_ids=[], chunk_size=None): bugs = super().get_bugs(date, bug_ids, chunk_size) diff --git a/templates/inactive_patch_author.html b/templates/inactive_patch_author.html new file mode 100644 index 000000000..6ff4b157e --- /dev/null +++ b/templates/inactive_patch_author.html @@ -0,0 +1,31 @@ +

The following {{ plural('bug has', data, pword='bugs have') }} patches with inactive authors:

+ + + + + + + + + + {% for i, (bugid, summary, revisions) in enumerate(data) -%} + + + + + + {% endfor -%} + +
BugSummaryPatches
+ {{ bugid }} + {{ summary | e }} + +
From 4603b451dfcb13595f5955fcba038db25421ed4e Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 25 Jun 2024 11:41:02 -0400 Subject: [PATCH 05/18] Added unassignment --- bugbot/rules/inactive_patch_author.py | 23 +++++++++++++++++++++-- templates/inactive_patch_author.html | 6 +++--- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index c88bb0a54..58b5d64bd 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -9,7 +9,7 @@ from libmozdata.phabricator import PhabricatorAPI from tenacity import retry, stop_after_attempt, wait_exponential -from bugbot import utils +from bugbot import people, utils from bugbot.bzcleaner import BzCleaner from bugbot.user_activity import PHAB_CHUNK_SIZE, UserActivity, UserStatus @@ -23,12 +23,14 @@ def __init__(self): super(InactivePatchAuthors, self).__init__() self.phab = PhabricatorAPI(utils.get_login_info()["phab_api_key"]) self.user_activity = UserActivity(include_fields=["nick"], phab=self.phab) + self.default_assignees = utils.get_default_assignees() + self.people = people.People.get_instance() def description(self): return "Bugs with inactive patch authors" def columns(self): - return ["id", "summary", "revisions"] + return ["id", "summary"] def get_bugs(self, date="today", bug_ids=[], chunk_size=None): bugs = super().get_bugs(date, bug_ids, chunk_size) @@ -44,12 +46,27 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): if inactive_patches: bug["inactive_patches"] = inactive_patches + self.unassign_inactive_author(bugid, bug, inactive_patches) print(f"Bug {bugid} has inactive patches: {inactive_patches}") else: del bugs[bugid] return bugs + def unassign_inactive_author(self, bugid, bug, inactive_patches): + # print(f"\n BUG >> {bugid} >> {bug}\n") + # prod = bug["product"] + # comp = bug["component"] + default_assignee = "benjaminmah2004@gmail.com" + autofix = {"assigned_to": default_assignee} + + comment = ( + "The patch author is inactive on Bugzilla, so the assignee is being reset." + ) + autofix["comment"] = {"body": comment} + + self.autofix_changes[bugid] = autofix + def _get_inactive_patch_authors(self, rev_ids: list) -> Dict[int, dict]: revisions: List[dict] = [] @@ -128,6 +145,8 @@ def get_bz_params(self, date): "attachments.file_name", "attachments.content_type", "attachments.is_obsolete", + "product", + "component", ] params = { "include_fields": fields, diff --git a/templates/inactive_patch_author.html b/templates/inactive_patch_author.html index 6ff4b157e..5effb2eb5 100644 --- a/templates/inactive_patch_author.html +++ b/templates/inactive_patch_author.html @@ -8,7 +8,7 @@ - {% for i, (bugid, summary, revisions) in enumerate(data) -%} + {% for i, (bugid, summary) in enumerate(data) -%} @@ -16,7 +16,7 @@ {{ bugid }} {{ summary | e }} - + {% endfor -%} From 0ae701805fdc6a82940bec90049907df2aa91c2b Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Wed, 26 Jun 2024 10:26:56 -0400 Subject: [PATCH 06/18] Added nagging template and disabled bugmail --- bugbot/rules/inactive_patch_author.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index 58b5d64bd..f835952ab 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -11,12 +11,13 @@ from bugbot import people, utils from bugbot.bzcleaner import BzCleaner +from bugbot.nag_me import Nag from bugbot.user_activity import PHAB_CHUNK_SIZE, UserActivity, UserStatus PHAB_FILE_NAME_PAT = re.compile(r"phabricator-D([0-9]+)-url\.txt") -class InactivePatchAuthors(BzCleaner): +class InactivePatchAuthors(BzCleaner, Nag): """Bugs with patches authored by inactive patch authors""" def __init__(self): @@ -25,6 +26,7 @@ def __init__(self): self.user_activity = UserActivity(include_fields=["nick"], phab=self.phab) self.default_assignees = utils.get_default_assignees() self.people = people.People.get_instance() + self.no_bugmail = True def description(self): return "Bugs with inactive patch authors" @@ -47,6 +49,7 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): if inactive_patches: bug["inactive_patches"] = inactive_patches self.unassign_inactive_author(bugid, bug, inactive_patches) + self.add([bug["assigned_to"], bug["triage_owner"]], bug) print(f"Bug {bugid} has inactive patches: {inactive_patches}") else: del bugs[bugid] @@ -54,7 +57,7 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): return bugs def unassign_inactive_author(self, bugid, bug, inactive_patches): - # print(f"\n BUG >> {bugid} >> {bug}\n") + print(f"\n BUG >> {bugid} >> {bug}\n") # prod = bug["product"] # comp = bug["component"] default_assignee = "benjaminmah2004@gmail.com" From 20e192f6a7bc042d57c05172b2fe6257359d33dd Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Wed, 26 Jun 2024 15:16:03 -0400 Subject: [PATCH 07/18] Reverted changes --- bugbot/rules/inactive_patch_author.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index f835952ab..622ebe4cf 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -2,6 +2,7 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this file, # You can obtain one at http://mozilla.org/MPL/2.0/. +import logging import re from typing import Dict, List @@ -11,13 +12,13 @@ from bugbot import people, utils from bugbot.bzcleaner import BzCleaner -from bugbot.nag_me import Nag from bugbot.user_activity import PHAB_CHUNK_SIZE, UserActivity, UserStatus +logging.basicConfig(level=logging.DEBUG) PHAB_FILE_NAME_PAT = re.compile(r"phabricator-D([0-9]+)-url\.txt") -class InactivePatchAuthors(BzCleaner, Nag): +class InactivePatchAuthors(BzCleaner): """Bugs with patches authored by inactive patch authors""" def __init__(self): @@ -26,7 +27,6 @@ def __init__(self): self.user_activity = UserActivity(include_fields=["nick"], phab=self.phab) self.default_assignees = utils.get_default_assignees() self.people = people.People.get_instance() - self.no_bugmail = True def description(self): return "Bugs with inactive patch authors" @@ -36,6 +36,8 @@ def columns(self): def get_bugs(self, date="today", bug_ids=[], chunk_size=None): bugs = super().get_bugs(date, bug_ids, chunk_size) + + logging.info(f"BUGS RETRIEVED > {bugs}") rev_ids = {rev_id for bug in bugs.values() for rev_id in bug["rev_ids"]} inactive_authors = self._get_inactive_patch_authors(list(rev_ids)) @@ -49,7 +51,6 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): if inactive_patches: bug["inactive_patches"] = inactive_patches self.unassign_inactive_author(bugid, bug, inactive_patches) - self.add([bug["assigned_to"], bug["triage_owner"]], bug) print(f"Bug {bugid} has inactive patches: {inactive_patches}") else: del bugs[bugid] @@ -57,10 +58,10 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): return bugs def unassign_inactive_author(self, bugid, bug, inactive_patches): - print(f"\n BUG >> {bugid} >> {bug}\n") + # print(f"\n BUG >> {bugid} >> {bug}\n") # prod = bug["product"] # comp = bug["component"] - default_assignee = "benjaminmah2004@gmail.com" + default_assignee = "bmah@mozilla.com" autofix = {"assigned_to": default_assignee} comment = ( From c48d00906a58d4fd9a4c1d8c66143890a27104f8 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Wed, 26 Jun 2024 16:41:56 -0400 Subject: [PATCH 08/18] Added product and component into data --- bugbot/rules/inactive_patch_author.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index 622ebe4cf..358cbe0ab 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -37,7 +37,6 @@ def columns(self): def get_bugs(self, date="today", bug_ids=[], chunk_size=None): bugs = super().get_bugs(date, bug_ids, chunk_size) - logging.info(f"BUGS RETRIEVED > {bugs}") rev_ids = {rev_id for bug in bugs.values() for rev_id in bug["rev_ids"]} inactive_authors = self._get_inactive_patch_authors(list(rev_ids)) @@ -58,10 +57,9 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): return bugs def unassign_inactive_author(self, bugid, bug, inactive_patches): - # print(f"\n BUG >> {bugid} >> {bug}\n") - # prod = bug["product"] - # comp = bug["component"] - default_assignee = "bmah@mozilla.com" + prod = bug["product"] + comp = bug["component"] + default_assignee = self.default_assignees[prod][comp] autofix = {"assigned_to": default_assignee} comment = ( @@ -139,6 +137,8 @@ def handle_bug(self, bug, data): bugid = str(bug["id"]) data[bugid] = { "rev_ids": rev_ids, + "product": bug["product"], + "component": bug["component"], } return bug @@ -151,6 +151,8 @@ def get_bz_params(self, date): "attachments.is_obsolete", "product", "component", + "assigned_to", + "triage_owner", ] params = { "include_fields": fields, From a25e5e9a79960d3a42fad6f7a345a54786cc79db Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Thu, 27 Jun 2024 10:30:11 -0400 Subject: [PATCH 09/18] Removed sending bugmail and added nagging --- bugbot/rules/inactive_patch_author.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index 358cbe0ab..67a28f507 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -12,13 +12,14 @@ from bugbot import people, utils from bugbot.bzcleaner import BzCleaner +from bugbot.nag_me import Nag from bugbot.user_activity import PHAB_CHUNK_SIZE, UserActivity, UserStatus logging.basicConfig(level=logging.DEBUG) PHAB_FILE_NAME_PAT = re.compile(r"phabricator-D([0-9]+)-url\.txt") -class InactivePatchAuthors(BzCleaner): +class InactivePatchAuthors(BzCleaner, Nag): """Bugs with patches authored by inactive patch authors""" def __init__(self): @@ -27,6 +28,7 @@ def __init__(self): self.user_activity = UserActivity(include_fields=["nick"], phab=self.phab) self.default_assignees = utils.get_default_assignees() self.people = people.People.get_instance() + self.no_bugmail = True def description(self): return "Bugs with inactive patch authors" @@ -51,11 +53,15 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): bug["inactive_patches"] = inactive_patches self.unassign_inactive_author(bugid, bug, inactive_patches) print(f"Bug {bugid} has inactive patches: {inactive_patches}") + self.add([bug["assigned_to"], bug["triage_owner"]], bug) else: del bugs[bugid] return bugs + def nag_template(self): + return self.name() + ".html" + def unassign_inactive_author(self, bugid, bug, inactive_patches): prod = bug["product"] comp = bug["component"] @@ -139,6 +145,8 @@ def handle_bug(self, bug, data): "rev_ids": rev_ids, "product": bug["product"], "component": bug["component"], + "assigned_to": bug["assigned_to"], + "triage_owner": bug["triage_owner"], } return bug From 38437a4d3bb46b572e3672852740aeda617c65c7 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Thu, 27 Jun 2024 14:46:17 -0400 Subject: [PATCH 10/18] Added phabricator patch abandoning --- bugbot/rules/inactive_patch_author.py | 64 ++++++++++++++++++--------- 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index 67a28f507..3c0a73af3 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -1,6 +1,6 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this file, -# You can obtain one at http://mozilla.org/MPL/2.0/. +# # This Source Code Form is subject to the terms of the Mozilla Public +# # License, v. 2.0. If a copy of the MPL was not distributed with this file, +# # You can obtain one at http://mozilla.org/MPL/2.0/. import logging import re @@ -40,7 +40,11 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): bugs = super().get_bugs(date, bug_ids, chunk_size) rev_ids = {rev_id for bug in bugs.values() for rev_id in bug["rev_ids"]} - inactive_authors = self._get_inactive_patch_authors(list(rev_ids)) + try: + inactive_authors = self._get_inactive_patch_authors(list(rev_ids)) + except Exception as e: + logging.error(f"Error fetching inactive patch authors: {e}") + inactive_authors = {} for bugid, bug in list(bugs.items()): inactive_patches = [ @@ -52,7 +56,6 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): if inactive_patches: bug["inactive_patches"] = inactive_patches self.unassign_inactive_author(bugid, bug, inactive_patches) - print(f"Bug {bugid} has inactive patches: {inactive_patches}") self.add([bug["assigned_to"], bug["triage_owner"]], bug) else: del bugs[bugid] @@ -73,33 +76,54 @@ def unassign_inactive_author(self, bugid, bug, inactive_patches): ) autofix["comment"] = {"body": comment} + # Abandon the patches + for patch in inactive_patches: + rev_id = patch["rev_id"] + try: + self.phab.request( + "differential.revision.edit", + objectIdentifier=rev_id, + transactions=[{"type": "abandon", "value": True}], + ) + logging.info(f"Abandoned patch {rev_id} for bug {bugid}.") + except Exception as e: + logging.error(f"Failed to abandon patch {rev_id} for bug {bugid}: {e}") + self.autofix_changes[bugid] = autofix def _get_inactive_patch_authors(self, rev_ids: list) -> Dict[int, dict]: revisions: List[dict] = [] for _rev_ids in Connection.chunks(rev_ids, PHAB_CHUNK_SIZE): - for revision in self._fetch_revisions(_rev_ids): - author_phid = revision["fields"]["authorPHID"] - created_at = revision["fields"]["dateCreated"] - if author_phid == "PHID-USER-eltrc7x5oplwzfguutrb": - continue - revisions.append( - { - "rev_id": revision["id"], - "author_phid": author_phid, - "created_at": created_at, - } - ) + try: + for revision in self._fetch_revisions(_rev_ids): + author_phid = revision["fields"]["authorPHID"] + created_at = revision["fields"]["dateCreated"] + if author_phid == "PHID-USER-eltrc7x5oplwzfguutrb": + continue + revisions.append( + { + "rev_id": revision["id"], + "author_phid": author_phid, + "created_at": created_at, + } + ) + except Exception as e: + logging.error(f"Error fetching revisions: {e}") + continue user_phids = set() for revision in revisions: user_phids.add(revision["author_phid"]) - users = self.user_activity.get_phab_users_with_status( - list(user_phids), keep_active=False - ) + try: + users = self.user_activity.get_phab_users_with_status( + list(user_phids), keep_active=False + ) + except Exception as e: + logging.error(f"Error fetching Phabricator users with status: {e}") + users = {} result: Dict[int, dict] = {} for revision in revisions: From efc6827395785c6b2e97c6be30a397cf638ffd25 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Thu, 27 Jun 2024 16:59:27 -0400 Subject: [PATCH 11/18] Added check for closed patches --- bugbot/rules/inactive_patch_author.py | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index 3c0a73af3..a016fb700 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -79,13 +79,26 @@ def unassign_inactive_author(self, bugid, bug, inactive_patches): # Abandon the patches for patch in inactive_patches: rev_id = patch["rev_id"] + revision = self.phab.request( + "differential.revision.search", + constraints={"ids": [rev_id]}, + )["data"][0] try: - self.phab.request( - "differential.revision.edit", - objectIdentifier=rev_id, - transactions=[{"type": "abandon", "value": True}], - ) - logging.info(f"Abandoned patch {rev_id} for bug {bugid}.") + if revision["fields"]["status"]["value"] in [ + "needs-review", + "needs-revision", + "accepted", + "changed-planned", + ]: + self.phab.request( + "differential.revision.edit", + objectIdentifier=rev_id, + transactions=[{"type": "abandon", "value": True}], + ) + logging.info(f"Abandoned patch {rev_id} for bug {bugid}.") + else: + logging.info(f"Patch {rev_id} for bug {bugid} is already closed.") + except Exception as e: logging.error(f"Failed to abandon patch {rev_id} for bug {bugid}: {e}") @@ -106,6 +119,7 @@ def _get_inactive_patch_authors(self, rev_ids: list) -> Dict[int, dict]: "rev_id": revision["id"], "author_phid": author_phid, "created_at": created_at, + "status": revision["fields"]["status"]["value"], } ) except Exception as e: From e5b4365b2829877d39bdbd65df20ad43ef5d9691 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Fri, 28 Jun 2024 12:52:10 -0400 Subject: [PATCH 12/18] Added exception handler for fetching user status from Phabricator --- bugbot/rules/inactive_patch_author.py | 14 +++------ bugbot/user_activity.py | 45 ++++++++++++++++----------- 2 files changed, 32 insertions(+), 27 deletions(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index a016fb700..3882c130b 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -112,8 +112,8 @@ def _get_inactive_patch_authors(self, rev_ids: list) -> Dict[int, dict]: for revision in self._fetch_revisions(_rev_ids): author_phid = revision["fields"]["authorPHID"] created_at = revision["fields"]["dateCreated"] - if author_phid == "PHID-USER-eltrc7x5oplwzfguutrb": - continue + # if author_phid == "PHID-USER-eltrc7x5oplwzfguutrb": + # continue revisions.append( { "rev_id": revision["id"], @@ -131,13 +131,9 @@ def _get_inactive_patch_authors(self, rev_ids: list) -> Dict[int, dict]: for revision in revisions: user_phids.add(revision["author_phid"]) - try: - users = self.user_activity.get_phab_users_with_status( - list(user_phids), keep_active=False - ) - except Exception as e: - logging.error(f"Error fetching Phabricator users with status: {e}") - users = {} + users = self.user_activity.get_phab_users_with_status( + list(user_phids), keep_active=False + ) result: Dict[int, dict] = {} for revision in revisions: diff --git a/bugbot/user_activity.py b/bugbot/user_activity.py index 13d3118d1..09dfaa7d7 100644 --- a/bugbot/user_activity.py +++ b/bugbot/user_activity.py @@ -2,6 +2,7 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this file, # You can obtain one at http://mozilla.org/MPL/2.0/. +import logging from datetime import timedelta from enum import Enum, auto from typing import Iterable, List, Optional @@ -15,6 +16,8 @@ from bugbot import utils from bugbot.people import People +logging.basicConfig(level=logging.DEBUG) + # The chunk size here should not be more than 100; which is the maximum number of # items that Phabricator could return in one response. PHAB_CHUNK_SIZE = 100 @@ -264,26 +267,32 @@ def get_phab_users_with_status( # will rely on the calendar from phab. for _user_phids in Connection.chunks(user_phids, PHAB_CHUNK_SIZE): for phab_user in self._fetch_phab_users(_user_phids): - user = users[phab_user["phid"]] - phab_status = self._get_status_from_phab_user(phab_user) - if phab_status: - user["status"] = phab_status - - elif user["status"] in ( - UserStatus.ABSENT, - UserStatus.INACTIVE, - ) and self.is_active_on_phab(phab_user["phid"]): - user["status"] = UserStatus.ACTIVE - - if not keep_active and user["status"] == UserStatus.ACTIVE: - del users[phab_user["phid"]] + try: + user = users[phab_user["phid"]] + phab_status = self._get_status_from_phab_user(phab_user) + if phab_status: + user["status"] = phab_status + + elif user["status"] in ( + UserStatus.ABSENT, + UserStatus.INACTIVE, + ) and self.is_active_on_phab(phab_user["phid"]): + user["status"] = UserStatus.ACTIVE + + if not keep_active and user["status"] == UserStatus.ACTIVE: + del users[phab_user["phid"]] + continue + + user["phab_username"] = phab_user["fields"]["username"] + user["unavailable_until"] = phab_user["attachments"][ + "availability" + ]["until"] + except Exception as e: + logging.error( + f"Error fetching inactive patch authors: '{phab_user['phid']}' - {str(e)}" + ) continue - user["phab_username"] = phab_user["fields"]["username"] - user["unavailable_until"] = phab_user["attachments"]["availability"][ - "until" - ] - return users def is_active_on_phab(self, user_phid: str) -> bool: From 413081690bea772e57038e7234b8d34a7966623e Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Fri, 28 Jun 2024 13:30:45 -0400 Subject: [PATCH 13/18] Removed comments in email template --- templates/inactive_patch_author.html | 9 --------- 1 file changed, 9 deletions(-) diff --git a/templates/inactive_patch_author.html b/templates/inactive_patch_author.html index 5effb2eb5..35b3647a1 100644 --- a/templates/inactive_patch_author.html +++ b/templates/inactive_patch_author.html @@ -16,15 +16,6 @@ {{ bugid }} {{ summary | e }} - {% endfor -%} From 243515d9bc4eac610e39a5cba6da1319a98bee3b Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 2 Jul 2024 11:14:34 -0400 Subject: [PATCH 14/18] Removed try-except block --- bugbot/rules/inactive_patch_author.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index 3882c130b..866d4fbe1 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -40,11 +40,8 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): bugs = super().get_bugs(date, bug_ids, chunk_size) rev_ids = {rev_id for bug in bugs.values() for rev_id in bug["rev_ids"]} - try: - inactive_authors = self._get_inactive_patch_authors(list(rev_ids)) - except Exception as e: - logging.error(f"Error fetching inactive patch authors: {e}") - inactive_authors = {} + + inactive_authors = self._get_inactive_patch_authors(list(rev_ids)) for bugid, bug in list(bugs.items()): inactive_patches = [ From 9eabe77d1e5aa28d6da66afa03ab3758597358b4 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 2 Jul 2024 11:46:51 -0400 Subject: [PATCH 15/18] Removed exception handling when fetching revisions --- bugbot/rules/inactive_patch_author.py | 30 ++++++++++++--------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index 866d4fbe1..7553f7a74 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -105,23 +105,19 @@ def _get_inactive_patch_authors(self, rev_ids: list) -> Dict[int, dict]: revisions: List[dict] = [] for _rev_ids in Connection.chunks(rev_ids, PHAB_CHUNK_SIZE): - try: - for revision in self._fetch_revisions(_rev_ids): - author_phid = revision["fields"]["authorPHID"] - created_at = revision["fields"]["dateCreated"] - # if author_phid == "PHID-USER-eltrc7x5oplwzfguutrb": - # continue - revisions.append( - { - "rev_id": revision["id"], - "author_phid": author_phid, - "created_at": created_at, - "status": revision["fields"]["status"]["value"], - } - ) - except Exception as e: - logging.error(f"Error fetching revisions: {e}") - continue + for revision in self._fetch_revisions(_rev_ids): + author_phid = revision["fields"]["authorPHID"] + created_at = revision["fields"]["dateCreated"] + # if author_phid == "PHID-USER-eltrc7x5oplwzfguutrb": + # continue + revisions.append( + { + "rev_id": revision["id"], + "author_phid": author_phid, + "created_at": created_at, + "status": revision["fields"]["status"]["value"], + } + ) user_phids = set() From f97642b8f698a4cb46603e4e69d62dd533cf8089 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 2 Jul 2024 11:51:26 -0400 Subject: [PATCH 16/18] Removed comment --- bugbot/rules/inactive_patch_author.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index 7553f7a74..8bdb2a014 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -108,8 +108,6 @@ def _get_inactive_patch_authors(self, rev_ids: list) -> Dict[int, dict]: for revision in self._fetch_revisions(_rev_ids): author_phid = revision["fields"]["authorPHID"] created_at = revision["fields"]["dateCreated"] - # if author_phid == "PHID-USER-eltrc7x5oplwzfguutrb": - # continue revisions.append( { "rev_id": revision["id"], From 3a8871655ffdd2a7745b17f038922c933c4f7dab Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 2 Jul 2024 12:12:27 -0400 Subject: [PATCH 17/18] Replaced `Exception` with `ConduitError` --- bugbot/rules/inactive_patch_author.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bugbot/rules/inactive_patch_author.py b/bugbot/rules/inactive_patch_author.py index 8bdb2a014..781d44247 100644 --- a/bugbot/rules/inactive_patch_author.py +++ b/bugbot/rules/inactive_patch_author.py @@ -7,7 +7,7 @@ from typing import Dict, List from libmozdata.connection import Connection -from libmozdata.phabricator import PhabricatorAPI +from libmozdata.phabricator import ConduitError, PhabricatorAPI from tenacity import retry, stop_after_attempt, wait_exponential from bugbot import people, utils @@ -96,7 +96,7 @@ def unassign_inactive_author(self, bugid, bug, inactive_patches): else: logging.info(f"Patch {rev_id} for bug {bugid} is already closed.") - except Exception as e: + except ConduitError as e: logging.error(f"Failed to abandon patch {rev_id} for bug {bugid}: {e}") self.autofix_changes[bugid] = autofix From d1b03508c66c325942ff3e5b5da158331d384d74 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 2 Jul 2024 12:26:17 -0400 Subject: [PATCH 18/18] Replaced `Exception` with `KeyError` --- bugbot/user_activity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bugbot/user_activity.py b/bugbot/user_activity.py index 09dfaa7d7..5a19bf9b4 100644 --- a/bugbot/user_activity.py +++ b/bugbot/user_activity.py @@ -287,7 +287,7 @@ def get_phab_users_with_status( user["unavailable_until"] = phab_user["attachments"][ "availability" ]["until"] - except Exception as e: + except KeyError as e: logging.error( f"Error fetching inactive patch authors: '{phab_user['phid']}' - {str(e)}" )