Docserver overhaul: Gitiles away from me.

Docserver overhaul: Gitiles away from me.

Minor update to the docserver. This is a multi-faceted patch which takes
care of some issues with our server:

 - No more AppEngine cron. The front-end is no longer responsible for
   updating Datastore entities, it just serves up the data that's there.

 - No more Gitiles or Subversion filesystems. Github filesystems are
   deprecated but not fully removed.

 - New LocalGitFileSystem can retrieve arbitrary repo history data
   directly from a local copy of the repo using commandline git.

 - Refactor appengine_wrappers; unused concepts deleted, datastore and
   URL fetching moved to their own first-class concepts, added
   environment_wrappers for more generic cross-platform factories.
   Memcache still lives in appengine_wrappers for now.

 - Clean up some of the mess caused by Gitiles support: no more refresh
   tasks, so no more refresh "paths" in data sources. Most of the
   foo_data_source.py changes here are just reverting that crap.
   FileSystem versioning is no longer necessary and no longer used, but
   not fully removed in this patch.

 - Added update_cache.py, a utility which can scrape a local chromium
   repo and populate Datastore with the data needed on the front-end.
   This tool can also be used to load and store on-disk caches of said
   Datastore in order to speed up incremental updates from the same
   host.

BUG=
R=kalman@chromium.org

Committed: https://crrev.com/0e461c740bf2004dc7a990da0bbc09e714446023
Cr-Commit-Position: refs/heads/master@{#333066}

[Ken Rockot(use gerrit already), 2015-05-26 00:26:25]

Hey, here's a big patch. Sorry. It's really not so bad though. I've annotated a
bunch of stuff with inline CL comments.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/PRESUBMIT.py (left):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/PRESUBMIT.py:69: cron_yaml_path =
os.path.join(input_api.PresubmitLocalPath(), 'cron.yaml')
There is no more cron.yaml, so...

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/admin_servlets.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/admin_servlets.py:54: class
FlushMemcacheServlet(Servlet):
This is necessary since we're pushing stuff into the Datastore from outside of
AppEngine and not going through a CacheChainObjectStore (because we can't use
AppEngine memcache from outside).

After every push, the CE instance queries /_flush_memcache on the docserver.

Not ideal, but also not too bad. In the future we can have a servlet that
invalidates specific entries, since each incremental update should only touch a
handful of keys in practice.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/admin_servlets.py:102: '''Devserver-only
servlet for pushing local file data into the datastore.
Unfortunately the AppEngine sandbox, even on dev_server, stubs out the
subprocess module making commandline git an impossible dream. It can however
read local files from disk. You can use update_cache.py to push updates to a
local file, then pull them into the Datastore with this query. See README for
details.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/admin_servlets_test.py (left):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/admin_servlets_test.py:54: def
testResetCommitServlet(self):
No longer exists.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/api_data_source.py:88: def Refresh(self):
Nothing interesting here, just merging all refresh work back into a single call.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/api_models.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/api_models.py:46: (self.name,
self.restrictedTo))
This is just a drive-by bugfix caught while hacking. Was obviously wrong.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/app.yaml (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/app.yaml:17: - url: /_flush_memcache/.*
Not protected behind login (as we can't login via a service account). Instead
protected by manual auth token validation.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/app_engine_handler.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/app_engine_handler.py:6: import webapp2
This is only ever loaded from within AppEngine now, so no sense in wrapping it.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/appengine_wrappers.py (left):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/appengine_wrappers.py:21: import
google.appengine.api.app_identity as app_identity
No longer used. Killed.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/appengine_wrappers.py:22: import
google.appengine.api.files as files
Used by github FS, but treating those as killed too.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/appengine_wrappers.py:23: import
google.appengine.api.logservice as logservice
Very little to wrap. Logic merged into custom_logger.py

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/appengine_wrappers.py:25: import
google.appengine.api.taskqueue as taskqueue
No longer used. Killed.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/appengine_wrappers.py:26: import
google.appengine.api.urlfetch as urlfetch
We now have a first class UrlFetcher interface. environment_wrappers defines a
CreateUrlFetcher() factory which just vends the right implementation based on
the running environment. Since there is now more than just "app engine / not app
engine" this made sense to me.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/appengine_wrappers.py:27: import
google.appengine.ext.blobstore as blobstore
No longer used except by github. Killed.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/appengine_wrappers.py:28: from
google.appengine.ext.blobstore.blobstore import BlobReferenceProperty
No longer used except by github. Killed.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/appengine_wrappers.py:29: import
google.appengine.ext.db as db
We have an (implied) PersistentObjectStore interface. It is implemented by both
PersistentObjectStoreFake and PersistentObjectStoreAppengine.
environment_wrappers provides a CreatePersistentObjectStore() method to get you
the right one.

When I broke this out, there was a third type (PersistentObjectStoreCloud), but
I've since killed it. We don't get async HTTP requests outside of appengine and
there are no stable or convenient Python libraries to get them, so a POS with
real Datastore backing was verrrry slow to use on the VM instance.

Now everything that isn't AppEngine uses a fake POS. Pushing into the real
Datastore is a separate task (see datastore_utils.py and update_cache.py for
usage).

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/appengine_wrappers.py:30: import webapp2
Only used in one place that's only run in AppEngine, so no wrapper necessary.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/caching_file_system.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/caching_file_system.py:8: import traceback
unused - removed in ps#2

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/caching_file_system.py:148:
logging.warn('Read cache miss for %s on %s' %
This was pretty useful in debugging, so I was going to leave it here.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/caching_file_system_test.py (left):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/caching_file_system_test.py:295: def
testVersionedStat(self):
No more versioned filesystems = no more support for versioned filesystems in
CachingFileSystem

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/content_provider.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/content_provider.py:84: try:
No logic changes here, just wrapping in a try block.

This prevents things like template syntax errors from breaking refresh, which
was happening before due to some }} somewhere in multidevice HTML.

Broken content will be replaced with empty content and log a warning.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/content_providers.py (left):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/content_providers.py:14: from
gitiles_file_system import GitilesFileSystem
\o/

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/content_providers.py:47:
github_file_system_provider,
I removed all the github FS plumbing for two reasons:

 1. It depends on blobstore and other AppEngine-only facilities
 2. Now that we have LocalGitFileSystem, the right (i.e. fast, reliable,
    consistent) way to support a git repo is to mirror it locally.

I didn't bother setting up mirrors for the existing repos because they have file
size issues, and those are conveniently abated by the live site automatically
falling through cache and serving directly from GCS.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/content_providers.py:190: config =
self._GetConfig()[path]
No net logic changes, just killing refresh paths

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/custom_logger.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/custom_logger.py:7: from environment
import IsAppEngine, IsComputeEngine
IsComputeEngine unused - removed in ps#2

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/datastore_models.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/datastore_models.py:6: import
google.appengine.ext.db as db
Only used from AppEngine, so this doesn't need a wrapper

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/datastore_models.py:37:
logging.warn('Refusing to create entity greater than 1 MB in size: %s/%s'
These entities would fail to push anyway. Failing here instead makes the error
easy to detect synchronously.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/datastore_util.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/datastore_util.py:3: # found in the
LICENSE file.
This provides the PushData function used exclusively by update_cache.py. In a
nutshell, it pushes a fake datastore into a real datastore. In practice, only
usable from a CE instance or if you locally set DOCSERVER_ACCESS_TOKEN to a
chrome-apps-doc service account token (but nobody should do that).

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/datastore_util.py:32: _MAX_REQUEST_SIZE =
5*1024*1024
Couldn't find any documentation for the max request size. 5 MB works
consistently.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/datastore_util.py:127: del data[k]
This should be super awesome and get us very tiny incremental updates (at which
point we should also do selective memcache eviction rather than flush_all).
However, one small problem: dicts do not retain key ordering across pickles. :(

So this is not always true:

  cPickle.dumps(cPickle.loads(cPickle.dumps(mydict))) == cPickle.dumps(mydict)

Because of this, every update still pushes about 1200 keys into the datastore.
We should fix it, but it doesn't seem like a major issue.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/docs_server_utils.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/docs_server_utils.py:61: return None
I was having a very bad time trying to track down an error somewhere deep in the
API processing code. I gave up and this was my compromise.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/environment.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/environment.py:35: return
_IsServerSoftware('Compute Engine')
Not a real thing, but there were no obvious environment variables I could glean
to ask the question synchronously and quickly. So we manually set
SERVER_SOFTWARE='Compute Engine' on the instance in order to make this work.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/environment.py:55: return sys.argv and
os.path.basename(sys.argv[0]).endswith('_test.py')
Added this because it's no longer true that !IsAppEngine() == (IsTest() ||
IsPreview())

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/gcs_file_system.py (left):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/gcs_file_system.py:118: def
_warnAboutAuthError(self):
Auth is no longer required. The github repos are public, so I made the buckets
public too. I think we can just avoid hosting docserver content in non-public
buckets.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/gcs_file_system.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/gcs_file_system.py:12: from
environment_wrappers import CreateUrlFetcher
Unfortunately cloudstorage_api only works from AppEngine and we needed GCS
support in other environments.

So I reimplemented CloudStorageFileSystem using the Cloud Storage JSON API
directly on top of whatever UrlFetcher the environment provides. This was pretty
trivial since we only need listing and file reading.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/gcs_file_system_provider.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/gcs_file_system_provider.py:12: from
environment_wrappers import CreateUrlFetcher
These are now unused. Removed in PS#2

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/gitiles_file_system.py (left):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/gitiles_file_system.py:2: # Use of this
source code is governed by a BSD-style license that can be
good riddance

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/instance_servlet.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/instance_servlet.py:17:
_DEVSERVER_DATASTORE_FILE = 'SERVER_DATA'
oops - removing this

[not at google - send to devlin, 2015-06-04 22:40:45]

This is the most complicated "Minor update" I've ever seem. lgtm - I admit I
haven't picked this apart, but I read it all, and I understand more or less how
it works, so cool :)

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/datastore_util.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/datastore_util.py:127: del data[k]
On 2015/05/26 00:26:24, Ken Rockot wrote:
> This should be super awesome and get us very tiny incremental updates (at
which
> point we should also do selective memcache eviction rather than flush_all).
> However, one small problem: dicts do not retain key ordering across pickles.
:(
> 
> So this is not always true:
> 
>   cPickle.dumps(cPickle.loads(cPickle.dumps(mydict))) == cPickle.dumps(mydict)
> 
> Because of this, every update still pushes about 1200 keys into the datastore.
> We should fix it, but it doesn't seem like a major issue.

What about an OrderedDict? That should pickle, and it should unpickle in the
same order. It's a PITA to need to use OrderedDict everywhere, but if it's super
awesome to get incremental updates, worth a shot.

or you can maintain a list on the side.

I haven't dived deep into this code, so I might be totally off here.

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/gitiles_file_system.py (left):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/gitiles_file_system.py:2: # Use of this
source code is governed by a BSD-style license that can be
On 2015/05/26 00:26:22, Ken Rockot wrote:
> good riddance

:-)

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/admin_servlets.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/admin_servlets.py:17:
_SERVICE_ACCOUNT_NAME = '636061184119-compute@developer.gserviceaccount.com'
Can you quickly describe what these are / where they come from?

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/api_data_source.py:98: return
All([get_platform_schemas(platform) for platform in GetPlatforms()])
btw you generally don't need to make this into a list, you can directly pass it
into All() as a generator. All(get_platform_schemas(platform) for platform in
GetPlatforms()).

(it would break if the implementation of All tries to iterate through the
generator more than once, but it doesn't.)

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/caching_file_system.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/caching_file_system.py:20: If the
underlying FileSystem is versioned (i.e., it implements GetVersion to
update this comment?

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/data_source.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/data_source.py:24: indicating the success
or failure of the refresh.'''
should probably be "which resolves to" not "indicating".

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/docs_server_utils.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/docs_server_utils.py:58: Returns None if
given None.
I would prefer if call sites checked for this. sensible to assert(data is not
None) though.

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/gcs_file_system.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/gcs_file_system.py:67: def
_CreateStatInfo(self, path):
What actually changed in this file?

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/local_git_file_system.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/local_git_file_system.py:39: # this
doesn't need to work anyway.
why would an AppEngine instance end up creating a LocalGitFileSystem?

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/update_cache.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/update_cache.py:140: '  --data-source=NAME
       Limit update to a single data source.\n'
isn't the opt library meant to generate this sort of thing itself? mm. never
mind.

[Ken Rockot(use gerrit already), 2015-06-05 00:21:50]

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/datastore_util.py (right):

https://codereview.chromium.org/1151283007/diff/1/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/datastore_util.py:127: del data[k]
On 2015/06/04 22:40:45, kalman wrote:
> On 2015/05/26 00:26:24, Ken Rockot wrote:
> > This should be super awesome and get us very tiny incremental updates (at
> which
> > point we should also do selective memcache eviction rather than flush_all).
> > However, one small problem: dicts do not retain key ordering across pickles.
> :(
> > 
> > So this is not always true:
> > 
> >   cPickle.dumps(cPickle.loads(cPickle.dumps(mydict))) ==
cPickle.dumps(mydict)
> > 
> > Because of this, every update still pushes about 1200 keys into the
datastore.
> > We should fix it, but it doesn't seem like a major issue.
> 
> What about an OrderedDict? That should pickle, and it should unpickle in the
> same order. It's a PITA to need to use OrderedDict everywhere, but if it's
super
> awesome to get incremental updates, worth a shot.
> 
> or you can maintain a list on the side.
> 
> I haven't dived deep into this code, so I might be totally off here.

No, I think that's the right approach. I didn't do OrderedDict in this CL
because it wasn't strictly necessary and it would make the CL even bigger. Nice
thing is, we can always move bits over to OrderedDict incrementally. I'll try it
with maybe CompiledFileSystem soon.

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/admin_servlets.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/admin_servlets.py:17:
_SERVICE_ACCOUNT_NAME = '636061184119-compute@developer.gserviceaccount.com'
On 2015/06/04 22:40:45, kalman wrote:
> Can you quickly describe what these are / where they come from?

Done.

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/api_data_source.py:98: return
All([get_platform_schemas(platform) for platform in GetPlatforms()])
On 2015/06/04 22:40:45, kalman wrote:
> btw you generally don't need to make this into a list, you can directly pass
it
> into All() as a generator. All(get_platform_schemas(platform) for platform in
> GetPlatforms()).
> 
> (it would break if the implementation of All tries to iterate through the
> generator more than once, but it doesn't.)

I think I ended up doing this intentionally to force eager evaluation of the
calls to GetSchemaView because it was helping me log and monitor refresh
performance per API. This could probably change to a generator, but I'm going to
leave it as-is for now...

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/caching_file_system.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/caching_file_system.py:20: If the
underlying FileSystem is versioned (i.e., it implements GetVersion to
On 2015/06/04 22:40:45, kalman wrote:
> update this comment?

DELETED!

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/data_source.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/data_source.py:24: indicating the success
or failure of the refresh.'''
On 2015/06/04 22:40:45, kalman wrote:
> should probably be "which resolves to" not "indicating".

Done.

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/docs_server_utils.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/docs_server_utils.py:58: Returns None if
given None.
On 2015/06/04 22:40:45, kalman wrote:
> I would prefer if call sites checked for this. sensible to assert(data is not
> None) though.

Upon further inspection the bug which was causing Nones to come in here has been
fixed, so I have happily removed this change altogether.

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/gcs_file_system.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/gcs_file_system.py:67: def
_CreateStatInfo(self, path):
On 2015/06/04 22:40:45, kalman wrote:
> What actually changed in this file?

It's using the GCS JSON API directly now instead of the (appengine-only)
cloudstorage_api module. It also assumes no need for access tokens since all the
buckets are public.

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/local_git_file_system.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/local_git_file_system.py:39: # this
doesn't need to work anyway.
On 2015/06/04 22:40:45, kalman wrote:
> why would an AppEngine instance end up creating a LocalGitFileSystem?

Because HostFileSystemProvider does it. We could stop that, but it then
complicates CachingFileSystem, since it uses the underlying FS's identity for
datastore key names.

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/update_cache.py (right):

https://codereview.chromium.org/1151283007/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/update_cache.py:140: '  --data-source=NAME
       Limit update to a single data source.\n'
On 2015/06/04 22:40:45, kalman wrote:
> isn't the opt library meant to generate this sort of thing itself? mm. never
> mind.
 
Ah you're right. Well sort of. It looks like argparse is the preferred solution
now. Which is good news, because I hate getopt. I will update this in a future
CL.

[Ken Rockot(use gerrit already), 2015-06-05 00:21:57]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2015-06-05 00:21:57]

The patchset sent to the CQ was uploaded after l-g-t-m from kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1151283007/#ps40001
(title: "address some comments")

[commit-bot: I haz the power, 2015-06-05 00:23:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1151283007/40001

[commit-bot: I haz the power, 2015-06-05 00:44:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-05 00:44:07]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ken Rockot(use gerrit already), 2015-06-05 16:26:55]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2015-06-05 16:26:55]

The patchset sent to the CQ was uploaded after l-g-t-m from kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1151283007/#ps60001
(title: "Remove inform_users template to fix presubmit failure (it's now a
redirect)")

[commit-bot: I haz the power, 2015-06-05 16:28:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1151283007/60001

[commit-bot: I haz the power, 2015-06-05 16:44:46]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-06-05 16:45:41]

Patchset 4 (id:??) landed as
https://crrev.com/0e461c740bf2004dc7a990da0bbc09e714446023
Cr-Commit-Position: refs/heads/master@{#333066}