Fix loading of periodic tasks and clean up extension loading.

This does a few things:

- add tests for extension loading
- refactor the extension and periodic task loading
- better handle assertions raised by extensions (e.g. when an extension tries to override an already registered view)
- attach exception traceback to error log during loading for improved debugging

bin/bundle-extensions

@@ -63,7 +63,7 @@ def load_bundles():
                 └── styles.css
 
     and would then need to register the bundle with an entry point
-    under the "redash.periodic_tasks" group, e.g. in your setup.py::
+    under the "redash.bundles" group, e.g. in your setup.py::
 
         setup(
             # ...

redash/extensions.py

@@ -13,45 +13,64 @@
 # after the configuration has already happened.
 periodic_tasks = odict()
 
-logger = logging.getLogger(__name__)
+extension_logger = logging.getLogger(__name__)
 
 
-def load_extensions(app):
-    """Load the Redash extensions for the given Redash Flask app.
-
-    The extension entry point can return any type of value but
-    must take a Flask application object.
-
-    E.g.::
-
-        def extension(app):
-            app.logger.info("Loading the Foobar extenions")
-            Foobar(app)
+def entry_point_loader(group_name, mapping, logger=None, *args, **kwargs):
+    """
+    Loads the list Python entry points with the given entry point group name
+    (e.g. "redash.extensions"), calls each with the provided *args/**kwargs
+    arguments and stores the results in the provided mapping under the name
+    of the entry point.
 
+    If provided, the logger is used for error and debugging statements.
     """
-    for entry_point in entry_points().get("redash.extensions", []):
-        app.logger.info('Loading Redash extension "%s".', entry_point.name)
+    if logger is None:
+        logger = extension_logger
+
+    for entry_point in entry_points().get(group_name, []):
+        logger.info('Loading entry point "%s".', entry_point.name)
         try:
             # Then try to load the entry point (import and getattr)
             obj = entry_point.load()
         except (ImportError, AttributeError):
             # or move on
-            app.logger.error(
-                'Redash extension "%s" could not be found.', entry_point.name
+            logger.error(
+                'Entry point "%s" could not be found.', entry_point.name, exc_info=True
             )
             continue
 
         if not callable(obj):
-            app.logger.error(
-                'Redash extension "%s" is not a callable.', entry_point.name
+            logger.error('Entry point "%s" is not a callable.', entry_point.name)
+            continue
+
+        try:
+            # then simply call the loaded entry point.
+            mapping[entry_point.name] = obj(*args, **kwargs)
+        except AssertionError:
+            logger.error(
+                'Entry point "%s" cound not be loaded.', entry_point.name, exc_info=True
             )
             continue
 
-        # then simply call the loaded entry point.
-        extensions[entry_point.name] = obj(app)
 
+def load_extensions(app):
+    """Load the Redash extensions for the given Redash Flask app.
+
+    The extension entry point can return any type of value but
+    must take a Flask application object.
+
+    E.g.::
+
+        def extension(app):
+            app.logger.info("Loading the Foobar extenions")
+            Foobar(app)
+
+    """
+    entry_point_loader("redash.extensions", extensions, logger=app.logger, app=app)
 
-def load_periodic_tasks(logger):
+
+def load_periodic_tasks(logger=None):
     """Load the periodic tasks as defined in Redash extensions.
 
     The periodic task entry point needs to return a set of parameters
@@ -82,21 +101,7 @@ def add_two_and_two():
             # ...
         )
     """
-    for entry_point in entry_points().get("redash.periodic_tasks", []):
-        logger.info(
-            'Loading periodic Redash tasks "%s" from "%s".',
-            entry_point.name,
-            entry_point.value,
-        )
-        try:
-            periodic_tasks[entry_point.name] = entry_point.load()
-        except (ImportError, AttributeError):
-            # and move on if it couldn't load it
-            logger.error(
-                'Periodic Redash task "%s" could not be found at "%s".',
-                entry_point.name,
-                entry_point.value,
-            )
+    entry_point_loader("redash.periodic_tasks", periodic_tasks, logger=logger)
 
 
 def init_app(app):

tests/extensions/redash-dummy/redash_dummy.py

@@ -0,0 +1,15 @@
+module_attribute = "hello!"
+
+
+def extension(app):
+    """This extension will work"""
+    return "extension loaded"
+
+
+def assertive_extension(app):
+    """This extension won't work"""
+    assert False
+
+
+def periodic_task(*args, **kwargs):
+    """This periodic task will successfully load"""

tests/extensions/redash-dummy/setup.py

@@ -0,0 +1,23 @@
+from setuptools import setup
+
+
+setup(
+    name="redash-dummy",
+    version="0.1",
+    description="Redash extensions for testing",
+    author="Redash authors",
+    license="MIT",
+    entry_points={
+        "redash.extensions": [
+            "working_extension = redash_dummy:extension",
+            "non_callable_extension = redash_dummy:module_attribute",
+            "not_findable_extension = redash_dummy:missing_attribute",
+            "not_importable_extension = missing_extension_module:extension",
+            "assertive_extension = redash_dummy:assertive_extension",
+        ],
+        "redash.periodic_tasks": [
+            "dummy_periodic_task = redash_dummy:periodic_task"
+        ],
+    },
+    py_modules=["redash_dummy"],
+)

tests/extensions/test_extensions.py

@@ -0,0 +1,48 @@
+import logging
+import os
+import subprocess
+
+from redash import extensions
+from tests import BaseTestCase
+
+logger = logging.getLogger(__name__)
+here = os.path.dirname(__file__)
+dummy_extension = "redash-dummy"
+
+
+class TestExtensions(BaseTestCase):
+    @classmethod
+    def setUpClass(cls):
+        dummy_path = os.path.join(here, dummy_extension)
+        subprocess.call(
+            ["pip", "install", "--disable-pip-version-check", "--user", "--editable", dummy_path]
+        )
+
+    @classmethod
+    def tearDownClass(cls):
+        subprocess.call(["pip", "uninstall", "--yes", dummy_extension])
+
+    def test_working_extension(self):
+        self.assertIn("working_extension", extensions.extensions.keys())
+        self.assertEqual(
+            extensions.extensions.get("working_extension"), "extension loaded"
+        )
+
+    def test_assertive_extension(self):
+        self.assertNotIn("assertive_extension", extensions.extensions.keys())
+
+    def test_not_findable_extension(self):
+        self.assertNotIn("not_findable_extension", extensions.extensions.keys())
+
+    def test_not_importable_extension(self):
+        self.assertNotIn("not_importable_extension", extensions.extensions.keys())
+
+    def test_non_callable_extension(self):
+        self.assertNotIn("non_callable_extension", extensions.extensions.keys())
+
+    def test_dummy_periodic_task(self):
+        # need to load the periodic tasks manually since this isn't
+        # done automatically on test suite start but only part of
+        # the worker configuration
+        extensions.load_periodic_tasks(logger)
+        self.assertIn("dummy_periodic_task", extensions.periodic_tasks.keys())