Extensions docs server: Design changes, partial template support.

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

 #!/usr/bin/env python

[Aaron Boodman, 2012/06/06 23:49:59]

Is the name 'echo_handler.py' still correct for this file?

 # 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 sys
 import os
 
 # Add the original server location to sys.path so we are able to import
 # modules from there.
 SERVER_PATH = 'chrome/common/extensions/docs/server2/'
 if os.path.abspath(SERVER_PATH) not in sys.path:
   sys.path.append(os.path.abspath(SERVER_PATH))
 
 import logging
 import urlfetch
 
 from google.appengine.ext import webapp
 from google.appengine.ext.webapp.util import run_wsgi_app
 
 from branch_utility import BranchUtility
 from local_fetcher import LocalFetcher
 from subversion_fetcher import SubversionFetcher
-from template_fetcher import TemplateFetcher
+from template_data_source import TemplateDataSource
 from template_utility import TemplateUtility
 
 EXTENSIONS_PATH = 'chrome/common/extensions/'
 DOCS_PATH = 'docs/'
 STATIC_PATH = DOCS_PATH + 'static/'
 PUBLIC_TEMPLATE_PATH = DOCS_PATH + 'template2/public/'
 PRIVATE_TEMPLATE_PATH = DOCS_PATH + 'template2/private/'
 
-class MainPage(webapp.RequestHandler):
-  def _FetchResource(self, fetcher, branch, path):
-    result = fetcher.FetchResource(branch, path)
-    for key in result.headers:
-      self.response.headers[key] = result.headers[key]
-    return result.content
+# Global cache of instances because the Server is recreated for every request.
+SERVER_INSTANCES = {}
+
+class Instance(object):

[Aaron Boodman, 2012/06/06 23:49:59]

Consider moving this into its own class. Easier to test.

[not at google - send to devlin, 2012/06/07 04:43:57]

Yes please. Might make sense to rename ServerInstance or something when you do.

[cduvall, 2012/06/08 00:39:23]

Done.

+  def __init__(self, data_source, fetcher):
+    self._data_source = data_source
+    self._fetcher = fetcher
+    self._template_util = TemplateUtility()

[not at google - send to devlin, 2012/06/07 04:43:57]

Usually it doesn't make sense to instantiate util classes, looking at
TemplateUtility, RenderTemplate (and any future util methods) should just be
def'd at the top level then imported without the "from".

[cduvall, 2012/06/08 00:39:23]

Done.

+
+  def Run(self, path, request_handler):
+    parts = path.split('/')
+    filename = parts[-1]
+    template = self._data_source[filename]
+    if template:
+      content = self._template_util.RenderTemplate(template,
+          '{"test": "Hello"}', self._data_source)
+    else:
+      logging.info('Template not found for: ' + filename)
+      try:
+        result = self._fetcher.FetchResource(STATIC_PATH + filename)
+        for key in result.headers:
+          request_handler.response.headers[key] = result.headers[key]
+        content = result.content
+      except:
+        # TODO should be an actual not found page.
+        content = 'File not found.'
+    request_handler.response.out.write(content)
+
+
+class Server(webapp.RequestHandler):
+  def __init__(self):
+    self._branch_util = BranchUtility(urlfetch)
+
+  def _GetInstanceForBranch(self, branch):
+    if branch in SERVER_INSTANCES:
+      return SERVER_INSTANCES[branch]
+    if branch == 'local':
+      fetcher = LocalFetcher(EXTENSIONS_PATH)
+      # No cache for local doc server.
+      cache_timeout = 0
+    else:
+      fetcher = SubversionFetcher(branch, EXTENSIONS_PATH, urlfetch)
+      cache_timeout = 300

[not at google - send to devlin, 2012/06/07 04:43:57]

nit: cache_timeout_seconds

[cduvall, 2012/06/08 00:39:23]

Done.

+    data_source = TemplateDataSource(fetcher,
+        [PUBLIC_TEMPLATE_PATH, PRIVATE_TEMPLATE_PATH], cache_timeout)

[not at google - send to devlin, 2012/06/07 04:43:57]

nit: parameters all on same line, or all on vertically aligned separate lines.
So, here... vertical obviously :)

[cduvall, 2012/06/08 00:39:23]

Done.

+    SERVER_INSTANCES[branch] = Instance(data_source, fetcher)
+    return SERVER_INSTANCES[branch]
+
+  def _HandleRequest(self, path):
+    channel_name = self._branch_util.GetChannelNameFromPath(path)

[not at google - send to devlin, 2012/06/07 04:43:57]

ditto comment about instantiating util classes.

[cduvall, 2012/06/08 00:39:23]

Done.

+    branch = self._branch_util.GetBranchNumberForChannelName(channel_name)
+
+    instance = self._GetInstanceForBranch(branch)
+    instance.Run(path, self)

[not at google - send to devlin, 2012/06/07 04:43:57]

nit: can inline instance

[cduvall, 2012/06/08 00:39:23]

Done.

 
   def get(self):
     path = self.request.path
     path = path.replace('/chrome/extensions/', '')
-    parts = path.split('/')
-    filename = parts[-1]
-
     if len(path) > 0 and path[0] == '/':
       path = path.strip('/')
-
-    branch_util = BranchUtility(urlfetch)
-    channel_name = branch_util.GetChannelNameFromPath(path)
-    branch = branch_util.GetBranchNumberForChannelName(channel_name)
-
-    if channel_name == 'local':
-      fetcher = LocalFetcher(EXTENSIONS_PATH)
-    else:
-      fetcher = SubversionFetcher(EXTENSIONS_PATH, urlfetch)
-    template_fetcher = TemplateFetcher(branch, fetcher)
-    template_util = TemplateUtility()
-
-    template = template_fetcher[PUBLIC_TEMPLATE_PATH + filename]
-    if template:
-      content = template_util.RenderTemplate(template, '{"test": "Hello"}')
-    else:
-      logging.info('Template for %s not found.' % filename)
-
-      try:
-        content = self._FetchResource(fetcher, branch, STATIC_PATH + filename)
-      except:
-        content = 'File not found.'
-
-    self.response.out.write(content)
+    self._HandleRequest(path)
 
 application = webapp.WSGIApplication([
-  ('/.*', MainPage),
+  ('/.*', Server),
 ], debug=False)
 
 def main():
   run_wsgi_app(application)

[not at google - send to devlin, 2012/06/07 04:43:57]

wondering if this a python or appengine idiom or something, but why create the
application outside main? could this be just

def main():
  handlers = [
    ('/*', Server)
  ]
  run_wsgi_app(webapp.WSGIApplication(handlers), debug=False)

btw why not debug=True, just curious?

[cduvall, 2012/06/08 00:39:23]

This is just how the old server was set up. I like your way better I'll change
it to that.

And I'm not sure what the debug setting will change, I just left it on
debug=False to limit unnecessary output.

 
 
 if __name__ == '__main__':
   main()