[Telemetry] Fix WPR files not being downloaded when required.

tools/telemetry/telemetry/wpr/archive_info.py

 # Copyright 2013 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 json
 import logging
 import os
 import re
 import shutil
 import tempfile
@@ skipping 55 matching lines @@
 
     Raises cloud storage Permissions or Credentials error when there is no
     local copy of the archive and the user doesn't have permission to access
     the archive's bucket.
 
     Warns when a bucket is not specified or when the user doesn't have
     permission to access the archive's bucket but a local copy of the archive
     exists.
     """
     # Download all .wpr files.
-    if self._data:

[aiolos (Not reviewing), 2015/01/13 19:42:58]

I believe this should actually be

if not self._data:

Because we assume self.data_ in the next code section.

[nednguyen, 2015/01/13 19:58:51]

lizeb@'s point is self._data cannot be None since FromFile is the constructor
method to be used. If it's just empty, then self._data['archives'] will throw
exception. Anyway, I asked lizeb@ to add the assertion that 'archives' in
self._data.

[aiolos (Not reviewing), 2015/01/13 20:18:57]

Ah, I missed that comment. We might rely on this not throwing exceptions on
archives with no data for some testing bits. If it fails in the CQ, we should
look into whether any of those assumption are valid, and may need to adjust some
tests.

The name of the function is slightly misleading if we added the assertion, and
should at the very least have it's doc updated to reflect that it will fail the
assert if self.data_ is empty.

-      return
     if not self._bucket:
       logging.warning('User story set in %s has no bucket specified, and '
                       'cannot be downloaded from cloud_storage.', )

[nednguyen, 2015/01/13 18:20:59]

Can you add: assert 'archives' in self._data, "Invalid data from file %s' %
self._file_path ?

[Benoit L, 2015/01/13 18:24:39]

Acknowledged.

The constructor would throw without 'archives' in self._data.

[nednguyen, 2015/01/13 18:38:14]

I prefer assertion since it gives a clearer debugging message. Also you get to
know the name of the file that contains invalid data that debug message.

-
     for archive_path in self._data['archives']:
       archive_path = self._WprFileNameToPath(archive_path)
       try:
         cloud_storage.GetIfChanged(archive_path, self._bucket)
       except (cloud_storage.CredentialsError, cloud_storage.PermissionError):
         if os.path.exists(archive_path):
           # If the archive exists, assume the user recorded their own and
           # simply warn.
           logging.warning('Need credentials to update WPR archive: %s',
                           archive_path)
@@ skipping 115 matching lines @@
 
   def _SetWprFileForUserStory(self, user_story_name, wpr_file):
     """For modifying the metadata when we're going to record a new archive."""
     old_wpr_file = self._user_story_name_to_wpr_file.get(user_story_name, None)
     if old_wpr_file:
       self._wpr_file_to_user_story_names[old_wpr_file].remove(user_story_name)
     self._user_story_name_to_wpr_file[user_story_name] = wpr_file
     if wpr_file not in self._wpr_file_to_user_story_names:
       self._wpr_file_to_user_story_names[wpr_file] = []
     self._wpr_file_to_user_story_names[wpr_file].append(user_story_name)