Docserver: Generate a table of extension/app API owners

chrome/common/extensions/docs/server2/owners_data_source.py

+# Copyright 2014 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+from operator import itemgetter
+import posixpath
+
+from data_source import DataSource
+from extensions_paths import BROWSER_API_PATHS, BROWSER_CHROME_EXTENSIONS
+from file_system import FileNotFoundError
+from path_util import IsDirectory
+from url_constants import CODEREVIEW_SERVER
+
+
+_COMMENT_START_MARKER = '#'
+_NO_OWNER = '<span class="warning">No owner.</span>'
+_OWNERS = 'OWNERS'
+_OWNER_BASE_URL = '<a href=' + CODEREVIEW_SERVER + '/user/%s>%s</a>'

[not at google - send to devlin, 2014/08/14 00:05:05]

Don't generate HTML here, generate enough data in the model to be able to
generate it in the template.

Maybe you'll need a noOwner flag and to pass CODEREVIEW_SERVER up to the
templates, etc.

+
+
+def _ParseOwnersFile(content):
+  '''Returns a tuple (owners, notes), where
+  |owners| is a string fromed from the owners in |content|,
+  |notes| is a string formed from the comments in |content|.
+  '''
+  if content is None:
+    return _NO_OWNER, 'See core extensions owners.'
+  owners = []
+  notes = []
+  for line in content.splitlines():
+    # TODO(ahernandez): Don't ignore per-file.
+    if line == '':
+      continue
+    if line.startswith(_COMMENT_START_MARKER):
+      notes.append(line[len(_COMMENT_START_MARKER):].lstrip())
+    else:
+      owners.append(line)
+  return ', '.join([_OWNER_BASE_URL % (owner, owner[:owner.find('@')])

[not at google - send to devlin, 2014/08/14 00:05:05]

You should be able to join() a generator (i.e. no need to surround in []).

+                    for owner in owners]), ' '.join(notes)
+
+
+class OwnersDataSource(DataSource):
+  def __init__(self, server_instance, _):
+    self._host_fs = server_instance.host_file_system_provider.GetTrunk()
+    self._cache = server_instance.object_store_creator.Create(OwnersDataSource)

[ahernandez, 2014/08/14 21:48:21]

The reason this is failing the cron servlet is that on the second cron run, this
cache here is recreated. Which means that the collect code below has to be run
all over again because the cache is empty, which results in several file system
reads. I'm not sure how to get around that. Is there a way to save the cache
between cron runs?

[not at google - send to devlin, 2014/08/14 22:02:10]

Hm, those object stores are supposed to stay around between tests, at least this
comment makes me believe so:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/exte...

but I don't think that's the issue, the test says that there should be the same
number of Stats but no re-reading because they shouldn't have changed. Might be
missing something.

How about let's try this with the Walk functionality, since that's an
improvement anyway, and take it from there?

[ahernandez, 2014/08/14 22:09:16]

I already have it implemented with Walk (I haven't uploaded another patchset
because I can't fix the cron_servlet_test). I have used logging statements to
verify that the caches indeed get reset, in both OwnersDataSource and
APIDataSource. However, I can't figure out why that's happening. Hopefully I
haven't been barking up the wrong tree. Should I upload the newest revision so
you can have a look at that?

+
+  def _CollectOwnersData(self):
+    '''Walks through the file system, collecting owners data from
+    API directories.
+    '''
+    def collect(api_owners):
+      if api_owners is not None:
+        return api_owners
+      api_owners = []
+      for root in BROWSER_API_PATHS:
+        for file_ in self._host_fs.ReadSingle(root).Get():

[not at google - send to devlin, 2014/08/14 00:05:05]

Then()

+          if not IsDirectory(file_):
+            continue
+          owners_file = posixpath.join(root, file_, _OWNERS)
+          content = self._host_fs.Read((owners_file,),
+                                       skip_not_found=True).Get()

[not at google - send to devlin, 2014/08/14 00:05:05]

Then()

+          owners, notes = _ParseOwnersFile(content.get(owners_file, None))
+          api_owners.append({
+            'apiName': file_[:-1],
+            'owners': owners,
+            'notes': notes
+          })
+      # Add an entry for the core extensions/apps owners.
+      extensions_owners = self._host_fs.ReadSingle(
+          posixpath.join(BROWSER_CHROME_EXTENSIONS, _OWNERS)).Get()

[not at google - send to devlin, 2014/08/14 00:05:05]

Then()

And splitting up these entirely serial methods into async blocks can be
challenging.

To simplify the logic here perhaps add a depth argument to FileSystem.Walk. Set
it to 1 for this method. Walk returns things like lists of files and that's
quite a lot more convenient than doing it by hand.

+      owners, notes = _ParseOwnersFile(extensions_owners)
+      api_owners.append({
+        'apiName': 'Core Extensions/Apps Owners',
+        'owners': owners,
+        'notes': notes
+      })
+      api_owners.sort(key=itemgetter('apiName'))
+      self._cache.Set('api_owners', api_owners)
+      return api_owners
+    return self._cache.Get('api_owners').Then(collect)
+    
+  def get(self, key):
+    return {
+      'apis': self._CollectOwnersData()
+    }.get(key).Get()
+
+  def Cron(self):
+    return self._CollectOwnersData()