owners-client: Add support for excluding owners.
It is useful to exclude the change author when suggesting owners.
Change-Id: I3321c013271f6cea1098abba59b509800818917e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2669680
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
diff --git a/owners_client.py b/owners_client.py
index 0d2f505..77d4d7d 100644
--- a/owners_client.py
+++ b/owners_client.py
@@ -104,12 +104,15 @@
status[path] = self.INSUFFICIENT_REVIEWERS
return status
- def ScoreOwners(self, paths):
+ def ScoreOwners(self, paths, exclude=None):
"""Get sorted list of owners for the given paths."""
+ exclude = exclude or []
positions_by_owner = {}
owners_by_path = self.BatchListOwners(paths)
for owners in owners_by_path.values():
for i, owner in enumerate(owners):
+ if owner in exclude:
+ continue
# Gerrit API lists owners of a path sorted by an internal score, so
# owners that appear first should be prefered.
# We define the score of an owner based on the pair
@@ -124,22 +127,26 @@
key=lambda o: (-len(positions_by_owner[o]),
min(positions_by_owner[o]) + random.random()))
- def SuggestOwners(self, paths):
+ def SuggestOwners(self, paths, exclude=None):
"""Suggest a set of owners for the given paths."""
+ exclude = exclude or []
paths_by_owner = {}
owners_by_path = self.BatchListOwners(paths)
for path, owners in owners_by_path.items():
for owner in owners:
- paths_by_owner.setdefault(owner, set()).add(path)
+ if owner not in exclude:
+ paths_by_owner.setdefault(owner, set()).add(path)
# Select the minimum number of owners that can approve all paths.
# We start at 2 to avoid sending all changes that require multiple
# reviewers to top-level owners.
- owners = self.ScoreOwners(paths)
+ owners = self.ScoreOwners(paths, exclude=exclude)
if len(owners) < 2:
return owners
- for num_owners in range(2, len(owners)):
+ # Note that we have to iterate up to len(owners) + 1.
+ # e.g. if there are only 2 owners, we should consider num_owners = 2.
+ for num_owners in range(2, len(owners) + 1):
# Iterate all combinations of `num_owners` by decreasing score, and
# select the first one that covers all paths.
for selected in _owner_combinations(owners, num_owners):
@@ -147,6 +154,8 @@
if len(covered) == len(paths):
return list(selected)
+ return []
+
class DepotToolsClient(OwnersClient):
"""Implement OwnersClient using owners.py Database."""
diff --git a/tests/owners_client_test.py b/tests/owners_client_test.py
index 4f727b3..cf9dfcc 100644
--- a/tests/owners_client_test.py
+++ b/tests/owners_client_test.py
@@ -166,6 +166,17 @@
)
self.client.owners_by_path = {
+ 'a': [alice, bob],
+ 'b': [bob],
+ 'c': [bob, chris]
+ }
+ self.assertEqual(
+ self.client.ScoreOwners(
+ self.client.owners_by_path.keys(), exclude=[chris]),
+ [bob, alice],
+ )
+
+ self.client.owners_by_path = {
'a': [alice, bob, chris, dave],
'b': [chris, bob, dave],
'c': [chris, dave],
@@ -187,6 +198,11 @@
sorted(self.client.SuggestOwners(['abcd'])),
[alice, bob])
+ self.client.owners_by_path = {'abcd': [alice, bob, chris, dave]}
+ self.assertEqual(
+ sorted(self.client.SuggestOwners(['abcd'], exclude=[alice, bob])),
+ [chris, dave])
+
self.client.owners_by_path = {
'ae': [alice, emily],
'be': [bob, emily],
