Die build.py, Die: Part 1 (BranchUtility and SubversionFetcher)

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

Index: chrome/common/extensions/docs/server2/branch_utility.py
diff --git a/chrome/common/extensions/docs/server2/branch_utility.py b/chrome/common/extensions/docs/server2/branch_utility.py
new file mode 100644
index 0000000000000000000000000000000000000000..9051e49da4bf437f583620a90e0de07c9aa231be
--- /dev/null
+++ b/chrome/common/extensions/docs/server2/branch_utility.py
@@ -0,0 +1,70 @@
+# Copyright (c) 2012 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.
+
+import logging
+import re
+import json
+
+from memcache import MemcacheAdd, MemcacheGet
+from fetch import Fetch
+
+class BranchUtility(object):
+  """Utility class for dealing with different doc branches.
+  """
+  def __init__(self):

[Aaron Boodman, 2012/05/22 22:45:02]

You could simplify this somewhat by passing the urlfetch module into the
BranchUtility constructor.

I have a feeling we are going to end up wanting to cache all external URLs that
we rely on, so we will create a wrapper for AppEngine's urlfetch that does
caching in memcache.

For now, you can just pass in the real urlfetch though. This will also make it
easy to unit test. Just pass in a fake urlfetch.

[cduvall, 2012/05/24 00:15:40]

Done.

+    self.url = 'http://omahaproxy.appspot.com/json'
+
+  def SetURL(self, url):
+    self.url = url
+
+  def _FetchURL(self, url):
+    return Fetch(url)
+
+  def GetChannelNameFromURL(self, url):

[Aaron Boodman, 2012/05/22 22:45:02]

I guess it's really GetChannelNameFromPath(), right? We're not dealing with full
URLs here.

[cduvall, 2012/05/24 00:15:40]

Done.

+    if url.startswith('dev'):

[Aaron Boodman, 2012/05/22 22:45:02]

startswith('dev/') ... same for the other branches.

[Aaron Boodman, 2012/05/22 22:45:02]

first_part = path.split('/')[0]
if first_part in ['trunk', 'dev', 'beta', 'stable']:
  return first_part
else:
  return 'stable'

[cduvall, 2012/05/24 00:15:40]

Done.

+      return 'dev'
+    if url.startswith('beta'):
+      return 'beta'
+    if url.startswith('trunk'):
+      return 'trunk'
+    return 'stable'
+
+  def GetBranchNumberForURL(self, url):
+    """Returns an empty string if the branch number cannot be found.
+    """
+    channel_name = self.GetChannelNameFromURL(url)
+    if channel_name == 'trunk':
+      return 'trunk'

[Aaron Boodman, 2012/05/22 22:45:02]

Still seems weird to accept trunk as an argument, but ok!

[cduvall, 2012/05/24 00:15:40]

I don't really like accepting trunk paths as an argument either, but some
channels have trunk as their branch. So the caller would have to have the trunk
vs. branch number logic twice (once before the call to filter out trunk paths,
and once after to filter them out again).

This way, they can pass any path into GetBranchNumber, and then do the trunk vs.
branch number logic after.

+
+    result = MemcacheGet(url, 'urls')

[Aaron Boodman, 2012/05/22 22:45:02]

With my change above, you wouldn't need this memcache stuff ... the urlfetch you
pass in would do it.

[cduvall, 2012/05/24 00:15:40]

Done.

+    if result is not None:
+      return result
+    logging.info('Branch number cache miss: ' + url)
+    fetch_data = self._FetchURL(self.url)
+
+    version_json = json.loads(fetch_data)
+    branch_numbers = {}
+    for entry in version_json:
+      if entry['os'] == 'cf':

[Aaron Boodman, 2012/05/22 22:45:02]

On second thought, let's be more robust and have a whitelist of platforms we
care about: linux, mac, win, and cros.

[cduvall, 2012/05/24 00:15:40]

Done.

+        continue
+      for version in entry['versions']:
+        if version['channel'] != channel_name:
+          continue
+        if version['true_branch'] not in branch_numbers:
+          branch_numbers[version['true_branch']] = 0
+        else:
+          branch_numbers[version['true_branch']] += 1
+
+    sorted_list = [x for x in branch_numbers.iteritems()]
+    sorted_list.sort(key = lambda x: x[1])
+    sorted_list.reverse()
+
+    branch_number = ''
+    if len(sorted_list) > 0:
+      branch_number, _ = sorted_list[0]
+
+    if not MemcacheAdd(url, branch_number, 'urls'):
+      logging.error('Memcache set failed.')
+
+    return branch_number