ソースを参照

Add BatchListBestOwners and ListBestOwners to GerritClient

ListOwners returns all OWNERS for a file, which is undesirable in cases
when only direct OWNERS are needed.

Bug: 1351212, 1351519
Change-Id: I693b6645c780aa589e8ab24d0b58691f4aeb30f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3823299
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Gavin Mak 3 年 前
コミット
e0fee9fa4b
3 ファイル変更44 行追加14 行削除
  1. 11 2
      gerrit_util.py
  2. 32 11
      owners_client.py
  3. 1 1
      tests/owners_client_test.py

+ 11 - 2
gerrit_util.py

@@ -874,14 +874,23 @@ def IsCodeOwnersEnabledOnRepo(host, repo):
   return not config['status'].get('disabled', False)
   return not config['status'].get('disabled', False)
 
 
 
 
-def GetOwnersForFile(host, project, branch, path, limit=100,
-                     resolve_all_users=True, seed=None, o_params=('DETAILS',)):
+def GetOwnersForFile(host,
+                     project,
+                     branch,
+                     path,
+                     limit=100,
+                     resolve_all_users=True,
+                     highest_score_only=False,
+                     seed=None,
+                     o_params=('DETAILS',)):
   """Gets information about owners attached to a file."""
   """Gets information about owners attached to a file."""
   path = 'projects/%s/branches/%s/code_owners/%s' % (
   path = 'projects/%s/branches/%s/code_owners/%s' % (
       urllib.parse.quote(project, ''),
       urllib.parse.quote(project, ''),
       urllib.parse.quote(branch, ''),
       urllib.parse.quote(branch, ''),
       urllib.parse.quote(path, ''))
       urllib.parse.quote(path, ''))
   q = ['resolve-all-users=%s' % json.dumps(resolve_all_users)]
   q = ['resolve-all-users=%s' % json.dumps(resolve_all_users)]
+  if highest_score_only:
+    q.append('highest-score-only=%s' % json.dumps(highest_score_only))
   if seed:
   if seed:
     q.append('seed=%d' % seed)
     q.append('seed=%d' % seed)
   if limit:
   if limit:

+ 32 - 11
owners_client.py

@@ -167,6 +167,7 @@ class GerritClient(OwnersClient):
     self._project = project
     self._project = project
     self._branch = branch
     self._branch = branch
     self._owners_cache = {}
     self._owners_cache = {}
+    self._best_owners_cache = {}
 
 
     # Seed used by Gerrit to shuffle code owners that have the same score. Can
     # Seed used by Gerrit to shuffle code owners that have the same score. Can
     # be used to make the sort order stable across several requests, e.g. to get
     # be used to make the sort order stable across several requests, e.g. to get
@@ -174,26 +175,46 @@ class GerritClient(OwnersClient):
     # same code owners.
     # same code owners.
     self._seed = random.getrandbits(30)
     self._seed = random.getrandbits(30)
 
 
-  def ListOwners(self, path):
+  def _FetchOwners(self, path, cache, highest_score_only=False):
     # Always use slashes as separators.
     # Always use slashes as separators.
     path = path.replace(os.sep, '/')
     path = path.replace(os.sep, '/')
-    if path not in self._owners_cache:
+    if path not in cache:
       # GetOwnersForFile returns a list of account details sorted by order of
       # GetOwnersForFile returns a list of account details sorted by order of
       # best reviewer for path. If owners have the same score, the order is
       # best reviewer for path. If owners have the same score, the order is
       # random, seeded by `self._seed`.
       # random, seeded by `self._seed`.
-      data = gerrit_util.GetOwnersForFile(
-          self._host, self._project, self._branch, path,
-          resolve_all_users=False, seed=self._seed)
-      self._owners_cache[path] = [
-        d['account']['email']
-        for d in data['code_owners']
-        if 'account' in d and 'email' in d['account']
+      data = gerrit_util.GetOwnersForFile(self._host,
+                                          self._project,
+                                          self._branch,
+                                          path,
+                                          resolve_all_users=False,
+                                          highest_score_only=highest_score_only,
+                                          seed=self._seed)
+      cache[path] = [
+          d['account']['email'] for d in data['code_owners']
+          if 'account' in d and 'email' in d['account']
       ]
       ]
       # If owned_by_all_users is true, add everyone as an owner at the end of
       # If owned_by_all_users is true, add everyone as an owner at the end of
       # the owners list.
       # the owners list.
       if data.get('owned_by_all_users', False):
       if data.get('owned_by_all_users', False):
-        self._owners_cache[path].append(self.EVERYONE)
-    return self._owners_cache[path]
+        cache[path].append(self.EVERYONE)
+    return cache[path]
+
+  def ListOwners(self, path):
+    return self._FetchOwners(path, self._owners_cache)
+
+  def ListBestOwners(self, path):
+    return self._FetchOwners(path,
+                             self._best_owners_cache,
+                             highest_score_only=True)
+
+  def BatchListBestOwners(self, paths):
+    """List only the higest-scoring owners for a group of files.
+
+    Returns a dictionary {path: [owners]}.
+    """
+    with git_common.ScopedPool(kind='threads') as pool:
+      return dict(
+          pool.imap_unordered(lambda p: (p, self.ListBestOwners(p)), paths))
 
 
 
 
 def GetCodeOwnersClient(root, upstream, host, project, branch):
 def GetCodeOwnersClient(root, upstream, host, project, branch):

+ 1 - 1
tests/owners_client_test.py

@@ -102,7 +102,7 @@ class GerritClientTest(unittest.TestCase):
     # Always use slashes as separators.
     # Always use slashes as separators.
     gerrit_util.GetOwnersForFile.assert_called_once_with(
     gerrit_util.GetOwnersForFile.assert_called_once_with(
         'host', 'project', 'branch', 'bar/everyone/foo.txt',
         'host', 'project', 'branch', 'bar/everyone/foo.txt',
-        resolve_all_users=False, seed=mock.ANY)
+        resolve_all_users=False, highest_score_only=False, seed=mock.ANY)
 
 
   def testListOwnersOwnedByAll(self):
   def testListOwnersOwnedByAll(self):
     mock.patch(
     mock.patch(