From 8441b337acaf8c17fee3c659bfa87e6d38f649f5 Mon Sep 17 00:00:00 2001
From: Zahari <zaharid@gmail.com>
Date: Wed, 8 Nov 2017 10:24:37 +0100
Subject: [PATCH] Change extra_providers to load scripts

Now the extra providers are loaded from a file path instead of an
import specification making them easier to use for causal tests.

There are a few open questions:

  - Whether we want to also have the old functionality of loading
    actual modules
  - How do the error messages look like when something fails in the
    imported provider?
  - Do we add a way to specify extra providers (module) in the config
    file? These are geared toward supporting something like
    `validphys.applgrid` where we do not commit to supporting some
    crazy setup by default.
---
 src/reportengine/app.py   | 22 ++++++++++++++++------
 src/reportengine/utils.py | 16 +++++++++++++++-
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/src/reportengine/app.py b/src/reportengine/app.py
index 0ed19b6..4583c45 100644
--- a/src/reportengine/app.py
+++ b/src/reportengine/app.py
@@ -21,7 +21,7 @@
 from reportengine.configparser import ConfigError, Config
 from reportengine.environment import Environment, EnvironmentError_
 from reportengine.baseexceptions import ErrorWithAlternatives
-from reportengine.utils import get_providers
+from reportengine.utils import get_providers, import_path
 from reportengine import colors
 from reportengine import helputils
 
@@ -175,8 +175,7 @@ def argparser(self):
                         default=('png', 'pdf',))
 
         parser.add_argument('-x', '--extra-providers', nargs='+',
-                            help="additional providers from which to "
-                            "load actions. Must be an importable specifiaction.")
+                            help="Python files to load additional providers from")
 
         parallel = parser.add_mutually_exclusive_group()
         parallel.add_argument('--parallel', action='store_true',
@@ -190,10 +189,20 @@ def argparser(self):
         return parser
 
     def init_providers(self, args):
-        extra_providers = args['extra_providers']
-        if extra_providers is None:
-            extra_providers = []
+        extra_provider_names = args['extra_providers']
+        try:
+            extra_providers = [import_path(p) for p in extra_provider_names]
+        except FileNotFoundError as e:
+            log.error(f"Could not import extra provider: No such file '{e}'.")
+            sys.exit(1)
+        except BaseException as e:
+            log.error("Error importing extra provider")
+
+            print(colors.color_exception(type(e), e, e.__traceback__), file=sys.stderr)
+            sys.exit(1)
+
         maybe_names = reversed(self.default_providers + extra_providers)
+
         providers = self.load_providers(maybe_names)
         self.providers = providers
 
@@ -207,6 +216,7 @@ def load_providers(self, maybe_names=None):
                     mod = importlib.import_module(mod)
                 except ImportError as e:
                     log.error("Could not import module %s", mod)
+                    #This *should* be a critical error.
                     raise
             providers.append(mod)
         return providers
diff --git a/src/reportengine/utils.py b/src/reportengine/utils.py
index 58e6b9a..f25f8d1 100644
--- a/src/reportengine/utils.py
+++ b/src/reportengine/utils.py
@@ -6,7 +6,9 @@
 import collections
 import pickle
 import inspect
+import pathlib
 import re
+import importlib
 
 def normalize_name(name):
     """Remove characters not suitable for filenames from the string"""
@@ -87,4 +89,16 @@ def ordinal(n):
         suffix = 'th'
     else:
         suffix = ('st', 'nd', 'rd')[residual - 1]
-    return '%d%s' % (n,suffix)
\ No newline at end of file
+    return '%d%s' % (n,suffix)
+
+def import_path(file_path):
+    """Load the module corresponding to a given path"""
+    #https://docs.python.org/3/library/importlib.html?highlight=import_module#importing-a-source-file-directly
+    file_path = pathlib.Path(file_path)
+    if not file_path.exists():
+        raise FileNotFoundError(file_path)
+    module_name = file_path.stem
+    spec = importlib.util.spec_from_file_location(module_name, file_path)
+    module = importlib.util.module_from_spec(spec)
+    spec.loader.exec_module(module)
+    return module