diff --git a/owners_client.py b/owners_client.py index 0d2f5052d..77d4d7d6e 100644 --- a/owners_client.py +++ b/owners_client.py @@ -104,12 +104,15 @@ class OwnersClient(object): 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 @@ class OwnersClient(object): 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 @@ class OwnersClient(object): 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 4f727b3a7..cf9dfcce9 100644 --- a/tests/owners_client_test.py +++ b/tests/owners_client_test.py @@ -165,6 +165,17 @@ class OwnersClientTest(unittest.TestCase): [bob, alice, chris] ) + 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], @@ -187,6 +198,11 @@ class OwnersClientTest(unittest.TestCase): 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],