Fix a few minor plugin-related TODOs

plugins/minio/tutorminio/patches/openedx-common-settings

@@ -2,6 +2,7 @@ AWS_S3_HOST = "{{ MINIO_HOST }}"
 AWS_S3_USE_SSL = {{ "True" if ACTIVATE_HTTPS else "False" }}
 AWS_S3_SECURE_URLS = {{ "True" if ACTIVATE_HTTPS else "False" }}
 AWS_S3_CALLING_FORMAT = "boto.s3.connection.OrdinaryCallingFormat"
+AWS_AUTO_CREATE_BUCKET = False # explicit is better than implicit
 
 # Configuring boto is required for ora2 because ora2 does not read
 # host/port/ssl settings from django. Hence this hack.

plugins/minio/tutorminio/templates/minio/scripts/minio-client/init

@@ -1,3 +1,2 @@
-# TODO we need to think long and hard whether we want to keep this init script or just set AWS_AUTO_CREATE_BUCKET=True
 mc config host add minio http://minio:9000 {{ OPENEDX_AWS_ACCESS_KEY }} {{ OPENEDX_AWS_SECRET_ACCESS_KEY }} --api s3v4
 mc mb --ignore-existing minio/{{ MINIO_BUCKET_NAME }} minio/{{ MINIO_FILE_UPLOAD_BUCKET_NAME }} minio/{{ MINIO_COURSE_IMPORT_EXPORT_BUCKET }}

tests/test_plugins.py

@@ -138,3 +138,13 @@ class PluginsTests(unittest.TestCase):
             self.assertEqual(
                 [("plugin1", "myclient")], list(plugins.iter_scripts({}, "init"))
             )
+    
+    def test_iter_templates(self):
+        class plugin1:
+            templates = "/tmp/templates"
+        with unittest.mock.patch.object(
+            plugins, "iter_enabled", return_value=[("plugin1", plugin1)]
+        ):
+            self.assertEqual(
+                [("plugin1", "/tmp/templates")], list(plugins.iter_templates({}))
+            )

tutor/commands/k8s.py

@@ -193,7 +193,6 @@ class K8sScriptRunner(scripts.BaseRunner):
         #     selector,
         #     "job",
         # )
-        # TODO check failure?
 
 
 k8s.add_command(quickstart)

tutor/config.py

@@ -118,7 +118,7 @@ def load_plugins(config, defaults):
     """
     for plugin_name, plugin in plugins.iter_enabled(config):
         plugin_prefix = plugin_name.upper() + "_"
-        plugin_config = plugins.get_callable_attr(plugin, "config")
+        plugin_config = plugins.get_callable_attr(plugin, "config", {})
 
         # Add new config key/values
         for key, value in plugin_config.get("add", {}).items():

tutor/env.py

@@ -22,9 +22,8 @@ class Renderer:
     def environment(cls, config):
         if not cls.ENVIRONMENT:
             template_roots = [TEMPLATES_ROOT]
-            for plugin_name, plugin in plugins.iter_enabled(config):
-                # TODO move this to plugins.iter_templates and add tests
-                template_roots.append(plugin.templates)
+            for _, plugin_templates in plugins.iter_templates(config):
+                template_roots.append(plugin_templates)
             environment = jinja2.Environment(
                 loader=jinja2.FileSystemLoader(template_roots),
                 undefined=jinja2.StrictUndefined,

tutor/plugins.py

@@ -50,17 +50,16 @@ class Patches:
     @classmethod
     def fill_cache(cls, config):
         for plugin_name, plugin in iter_enabled(config):
-            patches = get_callable_attr(plugin, "patches")
+            patches = get_callable_attr(plugin, "patches", {})
             for patch_name, content in patches.items():
                 if patch_name not in cls.CACHE:
                     cls.CACHE[patch_name] = {}
                 cls.CACHE[patch_name][plugin_name] = content
 
 
-def get_callable_attr(plugin, attr_name):
-    attr = getattr(plugin, attr_name, {})
+def get_callable_attr(plugin, attr_name, default=None):
+    attr = getattr(plugin, attr_name, default)
     if callable(attr):
-        # TODO pass config here for initialisation
         attr = attr()
     return attr
 
@@ -120,6 +119,12 @@ def iter_scripts(config, script_name):
     }
     """
     for plugin_name, plugin in iter_enabled(config):
-        scripts = get_callable_attr(plugin, "scripts")
+        scripts = get_callable_attr(plugin, "scripts", {})
         for service in scripts.get(script_name, []):
             yield plugin_name, service
+
+def iter_templates(config):
+    for plugin_name, plugin in iter_enabled(config):
+        templates = get_callable_attr(plugin, "templates")
+        if templates:
+            yield plugin_name, templates