Refactor serving_dirs.

tools/telemetry/telemetry/user_story/user_story_set.py

 # Copyright 2014 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 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
 
 
 class UserStorySet(object):
   """A collection of user story.
 
   A typical usage of UserStorySet would be to subclass it and then calling
   AddUserStory for each UserStory..
   """
 
   def __init__(self, archive_data_file='', cloud_storage_bucket=None,
                serving_dirs=None):
     """Creates a new UserStorySet.
 
     Args:
       archive_data_file: The path to Web Page Replay's archive data, relative
           to self.base_dir.
       cloud_storage_bucket: The cloud storage bucket used to download
           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
     self._wpr_archive_info = None
     archive_info.AssertValidCloudStorageBucket(cloud_storage_bucket)
     self._cloud_storage_bucket = cloud_storage_bucket
     self._base_dir = os.path.dirname(inspect.getfile(self.__class__))
     # Convert any relative serving_dirs to absolute paths.
     self._serving_dirs = set(os.path.realpath(os.path.join(self.base_dir, d))
                              for d in serving_dirs or [])
 
   @property
   def base_dir(self):
     """The base directory to resolve archive_data_file.
 
     This defaults to the directory containing the UserStorySet instance's class.
     """
     return self._base_dir
 
   @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):
     return self._archive_data_file
 
   @property
   def bucket(self):
     return self._cloud_storage_bucket
 
   @property
@@ skipping 39 matching lines @@
       user_story: The UserStory to lookup.
 
     Returns:
       The WPR archive file path for the given UserStory, if found.
       Otherwise, return None.
     """
     if not self.wpr_archive_info:
       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__()
 
   def __len__(self):
     return len(self.user_stories)
 
   def __getitem__(self, key):
     return self.user_stories[key]
 
   def __setitem__(self, key, value):
     self.user_stories[key] = value