Fix loading of periodic tasks and clean up extension loading. (#4064)

* 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

* Use site.addsitedir instead of calling pip.

* Use sys.path instead of site.addsitedir and also the setup.py egg_info command.

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/README.md

@@ -0,0 +1,22 @@
+# How to update the dummy extension?
+
+If you'd like to extend the dummy extension, please update the ``setup.py``
+file and the ``redash_dummy.py`` module.
+
+Please make sure to regenerate the *.egg-info directory. See below.
+
+# How to generate the redash_dummy.egg-info directory?
+
+The `egg-info` directory is what is usually created in the
+site-packages directory when running `pip install <packagename>` and
+contains the metadata derived from the `setup.py` file.
+
+In other words, it's auto-generated and you'll need to follow the following
+steps to update it (e.g. when extending the extension tests). From the
+host computer (assuming the Docker development environment) run:
+
+- `make bash` -- to create container running with Bash and entering it
+- `cd tests/extensions/redash-dummy/` -- change the directory to the directory with the dummy extension
+- `python setup.py egg_info` -- to create/update the egg-info directory
+
+The egg-info directory is *not* cleaned up by pip, just the link in the `~/.local` site-packages directory.

tests/extensions/redash-dummy/redash_dummy.egg-info/PKG-INFO

@@ -0,0 +1,10 @@
+Metadata-Version: 1.0
+Name: redash-dummy
+Version: 0.1
+Summary: Redash extensions for testing
+Home-page: UNKNOWN
+Author: Redash authors
+Author-email: UNKNOWN
+License: MIT
+Description: UNKNOWN
+Platform: UNKNOWN

tests/extensions/redash-dummy/redash_dummy.egg-info/SOURCES.txt

@@ -0,0 +1,8 @@
+README.md
+redash_dummy.py
+setup.py
+redash_dummy.egg-info/PKG-INFO
+redash_dummy.egg-info/SOURCES.txt
+redash_dummy.egg-info/dependency_links.txt
+redash_dummy.egg-info/entry_points.txt
+redash_dummy.egg-info/top_level.txt

tests/extensions/redash-dummy/redash_dummy.egg-info/dependency_links.txt

@@ -0,0 +1 @@
+

tests/extensions/redash-dummy/redash_dummy.egg-info/entry_points.txt

@@ -0,0 +1,10 @@
+[redash.extensions]
+assertive_extension = redash_dummy:assertive_extension
+non_callable_extension = redash_dummy:module_attribute
+not_findable_extension = redash_dummy:missing_attribute
+not_importable_extension = missing_extension_module:extension
+working_extension = redash_dummy:extension
+
+[redash.periodic_tasks]
+dummy_periodic_task = redash_dummy:periodic_task
+

tests/extensions/redash-dummy/redash_dummy.egg-info/top_level.txt

@@ -0,0 +1 @@
+redash_dummy

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,45 @@
+import logging
+import os
+import sys
+
+from redash import extensions
+from tests import BaseTestCase
+
+logger = logging.getLogger(__name__)
+dummy_extension = "redash-dummy"
+dummy_path = os.path.join(os.path.dirname(__file__), dummy_extension)
+
+
+class TestExtensions(BaseTestCase):
+    @classmethod
+    def setUpClass(cls):
+        sys.path.insert(0, dummy_path)
+
+    @classmethod
+    def tearDownClass(cls):
+        sys.path.remove(dummy_path)
+
+    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())