Docserver: Persist stat cache for versioned file systems

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

Index: chrome/common/extensions/docs/server2/file_system.py
diff --git a/chrome/common/extensions/docs/server2/file_system.py b/chrome/common/extensions/docs/server2/file_system.py
index c0c8551e88957237d16eb6fd3f8d88b96052e747..9e608febc52d61e94e272b3045e26555fb0ee758 100644
--- a/chrome/common/extensions/docs/server2/file_system.py
+++ b/chrome/common/extensions/docs/server2/file_system.py
@@ -159,14 +159,28 @@ class FileSystem(object):
     return Future(callback=lambda: self.Stat(path))
 
   def GetIdentity(self):
-    '''The identity of the file system, exposed for caching classes to
-    namespace their caches. this will usually depend on the configuration of
+    '''A legacy alias for the stable identity of the file system.'''
+    return self.GetStableIdentity()

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

(comment written before our chat, but I'll include it anyway)

If you did this rename in a pre-patch it'd make reviewing this patch easier
(fewer files changed), and mean that you don't need the GetIdentity() legacy
alias around.

You could even add that GetUnstableIdentity function which throws an
AssertionError for now, but just so that the comment exists to contrast with
GetStableIdentity.

+
+  def GetStableIdentity(self):
+    '''The stable identity of the file system, exposed for caching classes to
+    namespace their caches. This will usually depend on the configuration of
     that file system - e.g. a LocalFileSystem with a base path of /var is
     different to that of a SubversionFileSystem with a base path of /bar, is
     different to a LocalFileSystem with a base path of /usr.
     '''
     raise NotImplementedError(self.__class__)
 
+  def GetUnstableIdentity(self):

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

And yeah, it would be nice to think about these names and/or comments carefully.
I understand the difference of course (given our discussion yesterday) but I
don't imagine the names would help others.

May at least for now rewrite these comments and see how that might influence the
name?

As just discussed now, perhaps we should leave GetIdentity (but consider naming
it something less confusing) and add a GetVersion method, since only 2 things
care about it: CachingFileSystem and CompiledFileSystem. The equivalent, other
option would be to stick with the 2 different methods like you have, but now
we're back to the naming issue.

GetIdentity vs GetIdentityWithVersion?
GetLocation vs GetLocationWithVersion?

Maybe GetLocation would be a good (re)name for GetIdentity anyway.

+    '''The unstable identity of the file system, exposed for more volatile
+    caching classes to namespace their caches. This will usually depend not only
+    on the configuration of the file system, but also on its active revision
+    (e.g. a commit ID for a Git filesystem).
+
+    The default implementation is an alias for stable identity.
+    '''
+    return self.GetStableIdentity()
+
   def Walk(self, root, depth=-1, file_lister=None):
     '''Recursively walk the directories in a file system, starting with root.