Refactor serving_dirs.

tools/telemetry/telemetry/user_story/user_story_set.py

Index: tools/telemetry/telemetry/user_story/user_story_set.py
diff --git a/tools/telemetry/telemetry/user_story/user_story_set.py b/tools/telemetry/telemetry/user_story/user_story_set.py
index b4315ffc9e7799afe216fa37bcf115909d0daace..95cfc519b5780de487ce565e0387aad3dee826a1 100644
--- a/tools/telemetry/telemetry/user_story/user_story_set.py
+++ b/tools/telemetry/telemetry/user_story/user_story_set.py
@@ -5,7 +5,9 @@
 import inspect
 import os
 
+from telemetry import decorators
 from telemetry import user_story as user_story_module
+from telemetry.util import cloud_storage
 from telemetry.wpr import archive_info
 
 
@@ -27,6 +29,9 @@ class UserStorySet(object):
           Web Page Replay's archive data. Valid values are: None,
           PUBLIC_BUCKET, PARTNER_BUCKET, or INTERNAL_BUCKET (defined
           in telemetry.util.cloud_storage).
+      serving_dirs: A set of paths, relative to self.base_dir, to directories
+          containing hash files for non-wpr archive data stored in cloud
+          storage.
     """
     self.user_stories = []
     self._archive_data_file = archive_data_file
@@ -48,7 +53,11 @@ class UserStorySet(object):
 
   @property
   def serving_dirs(self):

[nednguyen, 2015/02/03 18:03:55]

Can you add unittest coverage for user_story_set.serving_dirs? Maybe use the
unittest in page_set for this one.

[aiolos (Not reviewing), 2015/02/19 21:29:39]

There already is in page_set_unittest. (I updated it in this CL already.) Is
there something in addition to the current test that you specifically want?

-    return self._serving_dirs
+    all_serving_dirs = self._serving_dirs.copy()
+    for user_story in self.user_stories:
+      if user_story.serving_dir:
+        all_serving_dirs.add(user_story.serving_dir)
+    return all_serving_dirs
 
   @property
   def archive_data_file(self):
@@ -108,6 +117,23 @@ class UserStorySet(object):
       return None
     return self.wpr_archive_info.WprFilePathForUserStory(story)
 
+  @decorators.Cache
+  def UpdateServingDirDataIfNeeded(self):

[nednguyen, 2015/02/03 01:37:37]

Why do you think it's better to move this functionality to here?

[aiolos (Not reviewing), 2015/02/03 02:35:36]

I definitely think we should move the function from it's current locations to
minimize maintenance and the possibility of code rot, but have no attachment to
it being here specifically. It might be better to put this in a util file, or
possibly add it to cloud_storage.py. Thoughts?

[nednguyen, 2015/02/03 18:03:55]

I am not sure I understand this argument, but it sounds ok to reduce the code
complexity in user_story_runner.
cloud_storage seems to be a better location for this. We can make a method
called UpdateServingDirData(serving_dir, bucket).

Please add unittest of it too. cloud_storage_unittest should already provide
some convenient ways for testing this without invoking real network.

[aiolos (Not reviewing), 2015/02/04 18:11:59]

cloud_storage actually doesn't seem to have unittests for any of it's important
functionality, and thus doesn't have anything setup to test it without touching
the network yet. There is cloud_storage_stub, which is a non-network touching
stub for other unittests to use. I was planning on adding a system stub version
of the function.

Should I look into a way to test the cloud_storage functions without touching
network?

[aiolos (Not reviewing), 2015/02/19 21:29:39]

Done.

+    # Scan every serving directory for .sha1 files
+    # and download them from Cloud Storage. Assume all data is public.
+    serving_dirs = self.serving_dirs
+    # Scan all serving dirs.
+    for serving_dir in serving_dirs:
+      if os.path.splitdrive(serving_dir)[1] == '/':
+        raise ValueError('Trying to serve root directory from HTTP server.')
+      for dirpath, _, filenames in os.walk(serving_dir):
+        for filename in filenames:
+          path, extension = os.path.splitext(
+              os.path.join(dirpath, filename))
+          if extension != '.sha1':
+            continue
+          cloud_storage.GetIfChanged(path, self.bucket)
+
   def __iter__(self):
     return self.user_stories.__iter__()