Revise error handling approach.

venmo · Nov 29, 2017 · 54baca1 · 54baca1
1 parent f1445e9
commit 54baca1
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 77 deletions.
diff --git a/distributed_nose/plugin.py b/distributed_nose/plugin.py
@@ -128,14 +128,9 @@ def configure(self, options, config):
             self.enabled = True
 
         if self.algorithm == self.ALGORITHM_LEAST_PROCESSING_TIME:
-            if not self.lpt_data_filepath:
-                logger.critical((
-                    "'--algorithm least-processing-time' requires "
-                    "'--lpt-data <lpt-data-filepath>' to be specified as "
-                    "well. Falling back to hash-ring algorithm."
-                ))
-                self.algorithm = self.ALGORITHM_HASH_RING
-            else:
+            assert self.lpt_data_filepath, "'--lpt-data' arg is set."
+
+            try:
                 # Set up the data structure for the nodes. Note that
                 # the 0th node is a dummy node. We do this since the nodes
                 # are 1-indexed and this prevents the need to do
@@ -148,51 +143,43 @@ def configure(self, options, config):
                     }
                     for _ in range(self.node_count + 1)
                 ]
-                try:
-                    with open(self.lpt_data_filepath) as f:
-                        self.lpt_data = json.load(f)
 
-                        # for now, lpt only operates at the class level
-                        self.hash_by_class = True
+                with open(self.lpt_data_filepath) as f:
+                    self.lpt_data = json.load(f)
 
-                        sorted_lpt_data = sorted(
-                            self.lpt_data.items(),
-                            key=lambda t: t[1]['duration'],
-                            reverse=True
-                        )
+                    # for now, lpt only operates at the class level
+                    self.hash_by_class = True
 
-                        for cls, data in sorted_lpt_data:
-                            node = min(
-                                self.lpt_nodes[1:],
-                                key=lambda n: n['processing_time']
-                            )
-                            node['processing_time'] += data['duration']
-                            node['classes'].add(cls)
-
-                except IOError:
-                    logger.critical(
-                        (
-                            "lpt-data file '%s' not found. "
-                            "Falling back to hash-ring algorithm."
-                        ),
-                        self.lpt_data_filepath
-                    )
-                    self.algorithm = self.ALGORITHM_HASH_RING
-                except ValueError as e:
-                    logger.critical(
-                        "%s. Falling back to hash-ring algorithm.",
-                        e
+                    sorted_lpt_data = sorted(
+                        self.lpt_data.items(),
+                        key=lambda t: t[1]['duration'],
+                        reverse=True
                     )
-                    self.algorithm = self.ALGORITHM_HASH_RING
-                except KeyError as e:
-                    logger.critical(
-                        (
-                            "%s. Invalid lpt data file. "
-                            "Falling back to hash-ring algorithm."
-                        ),
-                        e
-                    )
-                    self.algorithm = self.ALGORITHM_HASH_RING
+
+                    for cls, data in sorted_lpt_data:
+                        node = min(
+                            self.lpt_nodes[1:],
+                            key=lambda n: n['processing_time']
+                        )
+                        node['processing_time'] += data['duration']
+                        node['classes'].add(cls)
+
+            except IOError:
+                logger.critical(
+                    "lpt-data file '%s' not found. Aborting.",
+                    self.lpt_data_filepath
+                )
+                raise
+            except ValueError:
+                logger.critical(
+                    "Error decoding lpt-data file. Aborting."
+                )
+                raise
+            except KeyError:
+                logger.critical(
+                    "Invalid lpt data file. Aborting."
+                )
+                raise
 
         self.hash_ring = HashRing(range(1, self.node_count + 1))
 

diff --git a/tests/test_options.py b/tests/test_options.py
@@ -113,23 +113,19 @@ def test_lpt_via_flag(self):
         )
         self.assertTrue(self.plugin.enabled)
 
-    def test_lpt_no_data_arg_falls_back(self):
+    def test_lpt_no_data_arg_aborts(self):
         env = {'NOSE_NODES': 6,
                'NOSE_NODE_NUMBER': 4}
         self.plugin.options(self.parser, env=env)
         args = [
             '--algorithm=least-processing-time'
         ]
         options, _ = self.parser.parse_args(args)
-        self.plugin.configure(options, Config())
 
-        self.assertEqual(
-            self.plugin.algorithm,
-            DistributedNose.ALGORITHM_HASH_RING
-        )
-        self.assertTrue(self.plugin.enabled)
+        with self.assertRaises(AssertionError):
+            self.plugin.configure(options, Config())
 
-    def test_lpt_missing_data_file_falls_back(self):
+    def test_lpt_missing_data_file_aborts(self):
         LPT_DATA_FILEPATH = os.path.join(
             os.path.dirname(__file__),
             'no_such_file.json'
@@ -142,15 +138,11 @@ def test_lpt_missing_data_file_falls_back(self):
             '--lpt-data={}'.format(LPT_DATA_FILEPATH)
         ]
         options, _ = self.parser.parse_args(args)
-        self.plugin.configure(options, Config())
 
-        self.assertEqual(
-            self.plugin.algorithm,
-            DistributedNose.ALGORITHM_HASH_RING
-        )
-        self.assertTrue(self.plugin.enabled)
+        with self.assertRaises(IOError):
+            self.plugin.configure(options, Config())
 
-    def test_lpt_invalid_json_file_falls_back(self):
+    def test_lpt_invalid_json_file_aborts(self):
         LPT_DATA_FILEPATH = os.path.join(
             os.path.dirname(__file__),
             'lpt_data',
@@ -164,15 +156,11 @@ def test_lpt_invalid_json_file_falls_back(self):
             '--lpt-data={}'.format(LPT_DATA_FILEPATH)
         ]
         options, _ = self.parser.parse_args(args)
-        self.plugin.configure(options, Config())
 
-        self.assertEqual(
-            self.plugin.algorithm,
-            DistributedNose.ALGORITHM_HASH_RING
-        )
-        self.assertTrue(self.plugin.enabled)
+        with self.assertRaises(ValueError):
+            self.plugin.configure(options, Config())
 
-    def test_lpt_invalid_data_format_falls_back(self):
+    def test_lpt_invalid_data_format_aborts(self):
         LPT_DATA_FILEPATH = os.path.join(
             os.path.dirname(__file__),
             'lpt_data',
@@ -186,10 +174,6 @@ def test_lpt_invalid_data_format_falls_back(self):
             '--lpt-data={}'.format(LPT_DATA_FILEPATH)
         ]
         options, _ = self.parser.parse_args(args)
-        self.plugin.configure(options, Config())
 
-        self.assertEqual(
-            self.plugin.algorithm,
-            DistributedNose.ALGORITHM_HASH_RING
-        )
-        self.assertTrue(self.plugin.enabled)
+        with self.assertRaises(KeyError):
+            self.plugin.configure(options, Config())