refactor load_grid_manager() to better present errors (review)

src/allmydata/cli/grid_manager.py

@@ -54,7 +54,12 @@ def grid_manager(ctx, config):
         def grid_manager(self):
             if self._grid_manager is None:
                 config_path = _config_path_from_option(config)
-                self._grid_manager = load_grid_manager(config_path, config)
+                try:
+                    self._grid_manager = load_grid_manager(config_path)
+                except ValueError as e:
+                    raise click.ClickException(
+                        "Error loading Grid Manager from '{}': {}".format(config, e)
+                    )
             return self._grid_manager
 
     ctx.obj = Config()

src/allmydata/grid_manager.py

@@ -47,16 +47,16 @@ def create_grid_manager():
     )
 
 
-def load_grid_manager(config_path, config_location):
+def load_grid_manager(config_path):
     """
     Load a Grid Manager from existing configuration.
 
     :param FilePath config_path: the configuration location (or None for
         stdin)
 
-    :param str config_location: a string describing the config's location
-
     :returns: a GridManager instance
+
+    :raises: ValueError if the confguration is invalid
     """
     if config_path is None:
         config_file = sys.stdin
@@ -79,9 +79,7 @@ def load_grid_manager(config_path, config_location):
         )
     if 'private_key' not in config:
         raise ValueError(
-            "Grid Manager config from '{}' requires a 'private_key'".format(
+            "'private_key' required in config"
-                config_location,
-            )
         )
 
     private_key_bytes = config['private_key'].encode('ascii')

src/allmydata/test/test_grid_manager.py

@@ -139,7 +139,7 @@ class GridManagerVerifier(SyncTestCase):
         fp = FilePath(tempdir)
 
         save_grid_manager(fp, self.gm)
-        gm2 = load_grid_manager(fp, tempdir)
+        gm2 = load_grid_manager(fp)
         self.assertEqual(
             self.gm.public_identity(),
             gm2.public_identity(),
@@ -168,7 +168,7 @@ class GridManagerVerifier(SyncTestCase):
             json.dump(bad_config, f)
 
         with self.assertRaises(ValueError) as ctx:
-            load_grid_manager(fp, tempdir)
+            load_grid_manager(fp)
         self.assertIn(
             "unknown version",
             str(ctx.exception),
@@ -188,9 +188,9 @@ class GridManagerVerifier(SyncTestCase):
             json.dump(bad_config, f)
 
         with self.assertRaises(ValueError) as ctx:
-            load_grid_manager(fp, tempdir)
+            load_grid_manager(fp)
         self.assertIn(
-            "requires a 'private_key'",
+            "'private_key' required",
             str(ctx.exception),
         )
 
@@ -209,7 +209,7 @@ class GridManagerVerifier(SyncTestCase):
             json.dump(bad_config, f)
 
         with self.assertRaises(ValueError) as ctx:
-            load_grid_manager(fp, tempdir)
+            load_grid_manager(fp)
         self.assertIn(
             "Invalid Grid Manager private_key",
             str(ctx.exception),
@@ -234,7 +234,7 @@ class GridManagerVerifier(SyncTestCase):
             json.dump(bad_config, f)
 
         with self.assertRaises(ValueError) as ctx:
-            load_grid_manager(fp, tempdir)
+            load_grid_manager(fp)
         self.assertIn(
             "No 'public_key' for storage server",
             str(ctx.exception),