Decouple extensions from Flask app. (#3569)

* Decouple extensions from Flask app.

This separates the extension registry from the Flask app and also introduces a separate registry for preriodic tasks.

Fix #3466.

* Address review feedback.

* Update redash/extensions.py

Co-Authored-By: jezdez <jannis@leidel.info>

* Minor comment in requirements.

* Refactoring after getting feedback.

* Uncoupled bin/bundle-extensions from Flas app instance.

* Load bundles in bundle script and don’t rely on Flask.

* Upgraded to importlib-metadata 0.9.

* Add missing requirement.

* Fix TypeError.

* Added requirements for bundle_extension script.

* Install bundles requirement file correctly.

* Decouple bundle loading code from Redash.

* Install bundle requirements from requirements.txt.

* Use circleci/node for build-docker-image step, too.

.circleci/config.yml

@@ -39,7 +39,7 @@ jobs:
           name: Copy Test Results
           command: |
             mkdir -p /tmp/test-results/unit-tests
-            docker cp tests:/app/coverage.xml ./coverage.xml 
+            docker cp tests:/app/coverage.xml ./coverage.xml
             docker cp tests:/app/junit.xml /tmp/test-results/unit-tests/results.xml
       - store_test_results:
           path: /tmp/test-results
@@ -61,6 +61,7 @@ jobs:
     steps:
       - checkout
       - run: sudo apt install python-pip
+      - run: sudo pip install -r requirements_bundles.txt
       - run: npm install
       - run: npm run bundle
       - run: npm test
@@ -95,6 +96,7 @@ jobs:
     steps:
       - checkout
       - run: sudo apt install python-pip
+      - run: sudo pip install -r requirements_bundles.txt
       - run: npm install
       - run: .circleci/update_version
       - run: npm run bundle
@@ -105,11 +107,14 @@ jobs:
           path: /tmp/artifacts/
   build-docker-image:
     docker:
-      - image: circleci/buildpack-deps:xenial
+      - image: circleci/node:8
     steps:
       - setup_remote_docker
       - checkout
+      - run: sudo apt install python-pip
+      - run: sudo pip install -r requirements_bundles.txt
       - run: .circleci/update_version
+      - run: npm run bundle
       - run: .circleci/docker_build
 workflows:
   version: 2

Dockerfile

@@ -14,7 +14,7 @@ ARG skip_ds_deps
 
 # We first copy only the requirements file, to avoid rebuilding on every file
 # change.
-COPY requirements.txt requirements_dev.txt requirements_all_ds.txt ./
+COPY requirements.txt requirements_bundles.txt requirements_dev.txt requirements_all_ds.txt ./
 RUN pip install -r requirements.txt -r requirements_dev.txt
 RUN if [ "x$skip_ds_deps" = "x" ] ; then pip install -r requirements_all_ds.txt ; else echo "Skipping pip install -r requirements_all_ds.txt" ; fi
 

bin/bundle-extensions

@@ -1,39 +1,118 @@
 #!/usr/bin/env python
-
+# -*- coding: utf-8 -*-
+"""Copy bundle extension files to the client/app/extension directory"""
+import logging
 import os
-from subprocess import call
-from distutils.dir_util import copy_tree
+from pathlib2 import Path
+from shutil import copy
+from collections import OrderedDict as odict
+
+from importlib_metadata import entry_points
+from importlib_resources import contents, is_resource, path
 
-from pkg_resources import iter_entry_points, resource_filename, resource_isdir
+# Name of the subdirectory
+BUNDLE_DIRECTORY = "bundle"
 
+logger = logging.getLogger(__name__)
 
 
 # Make a directory for extensions and set it as an environment variable
 # to be picked up by webpack.
-EXTENSIONS_RELATIVE_PATH = os.path.join('client', 'app', 'extensions')
-EXTENSIONS_DIRECTORY = os.path.join(
-    os.path.dirname(os.path.dirname(__file__)),
-    EXTENSIONS_RELATIVE_PATH)
-
-if not os.path.exists(EXTENSIONS_DIRECTORY):
-    os.makedirs(EXTENSIONS_DIRECTORY)
-os.environ["EXTENSIONS_DIRECTORY"] = EXTENSIONS_RELATIVE_PATH
-
-for entry_point in iter_entry_points('redash.extensions'):
-    # This is where the frontend code for an extension lives
-    # inside of its package.
-    content_folder_relative = os.path.join(
-        entry_point.name, 'bundle')
-    (root_module, _) = os.path.splitext(entry_point.module_name)
-
-    if not resource_isdir(root_module, content_folder_relative):
-        continue
+extensions_relative_path = Path('client', 'app', 'extensions')
+extensions_directory = Path(__file__).parent.parent / extensions_relative_path
+
+if not extensions_directory.exists():
+    extensions_directory.mkdir()
+os.environ["EXTENSIONS_DIRECTORY"] = str(extensions_relative_path)
+
+
+def resource_isdir(module, resource):
+    """Whether a given resource is a directory in the given module
+
+    https://importlib-resources.readthedocs.io/en/latest/migration.html#pkg-resources-resource-isdir
+    """
+    try:
+        return resource in contents(module) and not is_resource(module, resource)
+    except (ImportError, TypeError):
+        # module isn't a package, so can't have a subdirectory/-package
+        return False
+
+
+def entry_point_module(entry_point):
+    """Returns the dotted module path for the given entry point"""
+    return entry_point.pattern.match(entry_point.value).group("module")
+
+
+def load_bundles():
+    """"Load bundles as defined in Redash extensions.
 
-    content_folder = resource_filename(root_module, content_folder_relative)
+    The bundle entry point can be defined as a dotted path to a module
+    or a callable, but it won't be called but just used as a means
+    to find the files under its file system path.
+
+    The name of the directory it looks for files in is "bundle".
+
+    So a Python package with an extension bundle could look like this::
+
+        my_extensions/
+        ├── __init__.py
+        └── wide_footer
+            ├── __init__.py
+            └── bundle
+                ├── extension.js
+                └── 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::
+
+        setup(
+            # ...
+            entry_points={
+                "redash.bundles": [
+                    "wide_footer = my_extensions.wide_footer",
+                ]
+                # ...
+            },
+            # ...
+        )
+
+    """
+    bundles = odict()
+    for entry_point in entry_points().get("redash.bundles", []):
+        logger.info('Loading Redash bundle "%s".', entry_point.name)
+        module = entry_point_module(entry_point)
+        # Try to get a list of bundle files
+        if not resource_isdir(module, BUNDLE_DIRECTORY):
+            logger.error(
+                'Redash bundle directory "%s" could not be found.', entry_point.name
+            )
+            continue
+        with path(module, BUNDLE_DIRECTORY) as bundle_dir:
+            bundles[entry_point.name] = list(bundle_dir.rglob("*"))
+
+    return bundles
+
+
+bundles = load_bundles().items()
+if bundles:
+    print('Number of extension bundles found: {}'.format(len(bundles)))
+else:
+    print('No extension bundles found.')
+
+for bundle_name, paths in bundles:
+    # Shortcut in case not paths were found for the bundle
+    if not paths:
+        print('No paths found for bundle "{}".'.format(bundle_name))
+        continue
 
-    # This is where we place our extensions folder.
-    destination = os.path.join(
-        EXTENSIONS_DIRECTORY,
-        entry_point.name)
+    # The destination for the bundle files with the entry point name as the subdirectory
+    destination = Path(extensions_directory, bundle_name)
+    if not destination.exists():
+        destination.mkdir()
 
-    copy_tree(content_folder, destination)
+    # Copy the bundle directory from the module to its destination.
+    print('Copying "{}" bundle to {}:'.format(bundle_name, destination.resolve()))
+    for src_path in paths:
+        dest_path = destination / src_path.name
+        print(" - {} -> {}".format(src_path, dest_path))
+        copy(str(src_path), str(dest_path))

redash/extensions.py

@@ -1,30 +1,103 @@
-import os
-from pkg_resources import iter_entry_points, resource_isdir, resource_listdir
+# -*- coding: utf-8 -*-
+import logging
+from collections import OrderedDict as odict
 
+from importlib_metadata import entry_points
 
-def init_app(app):
-    """
-    Load the Redash extensions for the given Redash Flask app.
-    """
-    if not hasattr(app, 'redash_extensions'):
-        app.redash_extensions = {}
+# The global Redash extension registry
+extensions = odict()
+
+# The periodic Celery tasks as provided by Redash extensions.
+# This is separate from the internal periodic Celery tasks in
+# celery_schedule since the extension task discovery phase is
+# after the configuration has already happened.
+periodic_tasks = odict()
+
+logger = logging.getLogger(__name__)
+
+
+def load_extensions(app):
+    """Load the Redash extensions for the given Redash Flask app.
 
-    for entry_point in iter_entry_points('redash.extensions'):
-        app.logger.info('Loading Redash extension %s.', entry_point.name)
+    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)
+
+    """
+    for entry_point in entry_points().get("redash.extensions", []):
+        app.logger.info('Loading Redash extension "%s".', entry_point.name)
         try:
-            extension = entry_point.load()
-            app.redash_extensions[entry_point.name] = {
-                "entry_function": extension(app),
-                "resources_list": []
+            # 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
+            )
+            continue
+
+        if not callable(obj):
+            app.logger.error(
+                'Redash extension "%s" is not a callable.', entry_point.name
+            )
+            continue
+
+        # then simply call the loaded entry point.
+        extensions[entry_point.name] = obj(app)
+
+
+def load_periodic_tasks(logger):
+    """Load the periodic tasks as defined in Redash extensions.
+
+    The periodic task entry point needs to return a set of parameters
+    that can be passed to Celery's add_periodic_task:
+
+        https://docs.celeryproject.org/en/latest/userguide/periodic-tasks.html#entries
+
+    E.g.::
+
+        def add_two_and_two():
+            return {
+                'name': 'add 2 and 2 every 10 seconds'
+                'sig': add.s(2, 2),
+                'schedule': 10.0,  # in seconds or a timedelta
             }
-        except ImportError:
-            app.logger.info('%s does not have a callable and will not be loaded.', entry_point.name)
-            (root_module, _) = os.path.splitext(entry_point.module_name)
-            content_folder_relative = os.path.join(entry_point.name, 'bundle')
-
-            # If it's a frontend extension only, store a list of files in the bundle directory.
-            if resource_isdir(root_module, content_folder_relative):
-                app.redash_extensions[entry_point.name] = {
-                    "entry_function": None,
-                    "resources_list": resource_listdir(root_module, content_folder_relative)
-                }
+
+    and then registered with an entry point under the "redash.periodic_tasks"
+    group, e.g. in your setup.py::
+
+        setup(
+            # ...
+            entry_points={
+                "redash.periodic_tasks": [
+                    "add_two_and_two = calculus.addition: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,
+            )
+
+
+def init_app(app):
+    load_extensions(app)

redash/worker.py

@@ -1,5 +1,4 @@
 from __future__ import absolute_import
-
 from datetime import timedelta
 from random import randint
 
@@ -8,15 +7,20 @@
 from celery import Celery
 from celery.schedules import crontab
 from celery.signals import worker_process_init
+from celery.utils.log import get_logger
 
-from redash import create_app, settings
+from redash import create_app, extensions, settings
 from redash.metrics import celery as celery_metrics  # noqa
 
 
+logger = get_logger(__name__)
+
+
 celery = Celery('redash',
                 broker=settings.CELERY_BROKER,
                 include='redash.tasks')
 
+# The internal periodic Celery tasks to automatically schedule.
 celery_schedule = {
     'refresh_queries': {
         'task': 'redash.tasks.refresh_queries',
@@ -71,18 +75,21 @@ def __call__(self, *args, **kwargs):
 celery.Task = ContextTask
 
 
-# Create Flask app after forking a new worker, to make sure no resources are shared between processes.
 @worker_process_init.connect
 def init_celery_flask_app(**kwargs):
+    """Create the Flask app after forking a new worker.
+
+    This is to make sure no resources are shared between processes.
+    """
     app = create_app()
     app.app_context().push()
 
 
-# Commented until https://github.com/getredash/redash/issues/3466 is implemented.
-# Hook for extensions to add periodic tasks.
-# @celery.on_after_configure.connect
-# def add_periodic_tasks(sender, **kwargs):
-#     app = safe_create_app()
-#     periodic_tasks = getattr(app, 'periodic_tasks', {})
-#     for params in periodic_tasks.values():
-#         sender.add_periodic_task(**params)
+@celery.on_after_configure.connect
+def add_periodic_tasks(sender, **kwargs):
+    """Load all periodic tasks from extensions and add them to Celery."""
+    # Populate the redash.extensions.periodic_tasks dictionary
+    extensions.load_periodic_tasks(logger)
+    for params in extensions.periodic_tasks.values():
+        # Add it to Celery's periodic task registry, too.
+        sender.add_periodic_task(**params)

requirements.txt

@@ -61,3 +61,7 @@ disposable-email-domains
 # It is not included by default because of the GPL license conflict.
 # ldap3==2.2.4
 gevent==1.4.0
+
+# Install the dependencies of the bin/bundle-extensions script here.
+# It has its own requirements file to simplify the frontend client build process
+-r requirements_bundles.txt