From a97d42970ffaba7b296a0c26ec21366de969a440 Mon Sep 17 00:00:00 2001 From: aiolos Date: Mon, 21 Sep 2015 17:04:22 -0700 Subject: [PATCH] Don't automatically default to using support_binaries in dependency_manager. BUG=510231 Review URL: https://codereview.chromium.org/1348303002 Cr-Commit-Position: refs/heads/master@{#350071} --- .../dependency_manager/dependency_manager.py | 28 +++++++++++++------ .../dependency_manager_unittest.py | 31 +++++++++++++++++----- .../catapult_base/dependency_manager/exceptions.py | 2 +- .../telemetry/internal/util/binary_manager.py | 6 +++-- 4 files changed, 50 insertions(+), 17 deletions(-) diff --git a/tools/telemetry/catapult_base/dependency_manager/dependency_manager.py b/tools/telemetry/catapult_base/dependency_manager/dependency_manager.py index d7395acf13e6..29f389f44249 100644 --- a/tools/telemetry/catapult_base/dependency_manager/dependency_manager.py +++ b/tools/telemetry/catapult_base/dependency_manager/dependency_manager.py @@ -53,7 +53,7 @@ class DependencyManager(object): for config in configs: self._UpdateDependencies(config) - def FetchPath(self, dependency, platform): + def FetchPath(self, dependency, platform, try_support_binaries=False): """Get a path to an executable for |dependency|, downloading as needed. A path to a default executable may be returned if a platform specific @@ -65,6 +65,8 @@ class DependencyManager(object): platform: Name of the platform the dependency will run on. Often of the form 'os_architecture'. Must match those specified in the config(s) used in this DependencyManager. + try_support_binaries: True if support_binaries should be queried if the + dependency_manager was not initialized with data for |dependency|. Returns: A path to an executable of |dependency| that will run on |platform|, @@ -87,8 +89,13 @@ class DependencyManager(object): """ dependency_info = self._GetDependencyInfo(dependency, platform) if not dependency_info: - # TODO(aiolos): Replace the support_binaries call with an error once all - # binary dependencies are moved over to the new system. + logging.error( + 'The dependency_manager was not initialized with the dependency.') + if not try_support_binaries: + raise exceptions.NoPathFoundError(dependency, platform) + # TODO(aiolos): Remove the support_binaries call and always raise + # NoPathFound once the binary dependencies are moved over to the new + # system. # platform should be of the form '%s_%s' % (os_name, arch_name) when # called from the binary_manager. @@ -100,8 +107,6 @@ class DependencyManager(object): platform_arch)) return support_binaries.FindPath(dependency, platform_arch, platform_os) - logging.info('Looking for dependency %s, on platform %s in the dependency ' - 'manager.' % (dependency, platform)) path = self._LocalPath(dependency_info) if not path or not os.path.exists(path): path = self._CloudStoragePath(dependency_info) @@ -109,7 +114,7 @@ class DependencyManager(object): raise exceptions.NoPathFoundError(dependency, platform) return path - def LocalPath(self, dependency, platform): + def LocalPath(self, dependency, platform, try_support_binaries=False): """Get a path to a locally stored executable for |dependency|. A path to a default executable may be returned if a platform specific @@ -122,6 +127,8 @@ class DependencyManager(object): platform: Name of the platform the dependency will run on. Often of the form 'os_architecture'. Must match those specified in the config(s) used in this DependencyManager. + try_support_binaries: True if support_binaries should be queried if the + dependency_manager was not initialized with data for |dependency|. Returns: A path to an executable for |dependency| that will run on |platform|. @@ -129,10 +136,15 @@ class DependencyManager(object): Raises: NoPathFoundError: If a local copy of the executable cannot be found. """ - # TODO(aiolos): Replace the support_binaries call with an error once all - # binary dependencies are moved over to the new system. + # TODO(aiolos): Remove the support_binaries call and always raise + # NoPathFound once the binary dependencies are moved over to the new + # system. dependency_info = self._GetDependencyInfo(dependency, platform) if not dependency_info: + logging.error( + 'The dependency_manager was not initialized with the dependency.') + if not try_support_binaries: + raise exceptions.NoPathFoundError(dependency, platform) return support_binaries.FindLocallyBuiltPath(dependency) local_path = self._LocalPath(dependency_info) if not local_path or not os.path.exists(local_path): diff --git a/tools/telemetry/catapult_base/dependency_manager/dependency_manager_unittest.py b/tools/telemetry/catapult_base/dependency_manager/dependency_manager_unittest.py index eadb70ab7edc..0a7ab0e52166 100644 --- a/tools/telemetry/catapult_base/dependency_manager/dependency_manager_unittest.py +++ b/tools/telemetry/catapult_base/dependency_manager/dependency_manager_unittest.py @@ -45,8 +45,9 @@ class DependencyManagerTest(unittest.TestCase): @mock.patch( 'catapult_base.dependency_manager.DependencyManager._CloudStoragePath') @mock.patch('catapult_base.dependency_manager.DependencyManager._LocalPath') - def testFetchPathSupportBinaries(self, local_path_mock, cs_path_mock, - dep_info_mock, sb_find_path_mock, path_mock): + def testFetchPathUnititializedDependency( + self, local_path_mock, cs_path_mock, dep_info_mock, sb_find_path_mock, + path_mock): dep_manager = dependency_manager.DependencyManager([]) self.assertFalse(local_path_mock.call_args) self.assertFalse(cs_path_mock.call_args) @@ -60,7 +61,12 @@ class DependencyManagerTest(unittest.TestCase): dep_info_mock.return_value = None # Empty lookup_dict - found_path = dep_manager.FetchPath('dep', 'plat_arch_x86') + with self.assertRaises(exceptions.NoPathFoundError): + dep_manager.FetchPath('dep', 'plat_arch_x86') + dep_info_mock.reset_mock() + + found_path = dep_manager.FetchPath( + 'dep', 'plat_arch_x86', try_support_binaries=True) self.assertEqual(sb_path, found_path) self.assertFalse(local_path_mock.call_args) self.assertFalse(cs_path_mock.call_args) @@ -75,8 +81,12 @@ class DependencyManagerTest(unittest.TestCase): # for. dep_manager._lookup_dict = {'dep1': mock.MagicMock(), 'dep2': mock.MagicMock()} - found_path = dep_manager.FetchPath('dep', 'plat_arch_x86') + with self.assertRaises(exceptions.NoPathFoundError): + dep_manager.FetchPath('dep', 'plat_arch_x86') + dep_info_mock.reset_mock() + found_path = dep_manager.FetchPath( + 'dep', 'plat_arch_x86', try_support_binaries=True) self.assertEqual(sb_path, found_path) self.assertFalse(local_path_mock.call_args) self.assertFalse(cs_path_mock.call_args) @@ -245,7 +255,12 @@ class DependencyManagerTest(unittest.TestCase): dep_info_mock.return_value = None # Empty lookup_dict - found_path = dep_manager.LocalPath('dep', 'plat') + with self.assertRaises(exceptions.NoPathFoundError): + dep_manager.LocalPath('dep', 'plat') + dep_info_mock.reset_mock() + + found_path = dep_manager.LocalPath( + 'dep', 'plat', try_support_binaries=True) self.assertEqual(sb_path, found_path) self.assertFalse(local_path_mock.call_args) sb_find_path_mock.assert_called_once_with('dep') @@ -258,8 +273,12 @@ class DependencyManagerTest(unittest.TestCase): # for. dep_manager._lookup_dict = {'dep1': mock.MagicMock(), 'dep2': mock.MagicMock()} - found_path = dep_manager.LocalPath('dep', 'plat') + with self.assertRaises(exceptions.NoPathFoundError): + dep_manager.LocalPath('dep', 'plat') + dep_info_mock.reset_mock() + found_path = dep_manager.LocalPath( + 'dep', 'plat', try_support_binaries=True) self.assertEqual(sb_path, found_path) self.assertFalse(local_path_mock.call_args) sb_find_path_mock.assert_called_once_with('dep') diff --git a/tools/telemetry/catapult_base/dependency_manager/exceptions.py b/tools/telemetry/catapult_base/dependency_manager/exceptions.py index 44ed452269dd..4d303aa5e7d6 100644 --- a/tools/telemetry/catapult_base/dependency_manager/exceptions.py +++ b/tools/telemetry/catapult_base/dependency_manager/exceptions.py @@ -22,7 +22,7 @@ class FileNotFoundError(Exception): super(FileNotFoundError, self).__init__('No file found at %s' % file_path) -class NoPathFoundError(FileNotFoundError): +class NoPathFoundError(Exception): def __init__(self, dependency, platform): super(NoPathFoundError, self).__init__( 'No file could be found locally, and no file to download from cloud ' diff --git a/tools/telemetry/telemetry/internal/util/binary_manager.py b/tools/telemetry/telemetry/internal/util/binary_manager.py index 4ea7db6af2c3..e277b7041ee1 100644 --- a/tools/telemetry/telemetry/internal/util/binary_manager.py +++ b/tools/telemetry/telemetry/internal/util/binary_manager.py @@ -44,7 +44,8 @@ def FetchPath(binary_name, arch, platform): if _dependency_manager is None: raise exceptions.InitializationError( 'Called FetchPath with uninitialized binary manager.') - return _dependency_manager.FetchPath(binary_name, '%s_%s' % (platform, arch)) + return _dependency_manager.FetchPath( + binary_name, '%s_%s' % (platform, arch), try_support_binaries=True) def LocalPath(binary_name, arch, platform): @@ -56,4 +57,5 @@ def LocalPath(binary_name, arch, platform): if _dependency_manager is None: raise exceptions.InitializationError( 'Called LocalPath with uninitialized binary manager.') - return _dependency_manager.LocalPath(binary_name, '%s_%s' % (platform, arch)) + return _dependency_manager.LocalPath( + binary_name, '%s_%s' % (platform, arch), try_support_binaries=True) -- 2.11.4.GIT