chore: address PR comments

overhangio · Nov 19, 2024 · 4e60cb3 · 4e60cb3
1 parent b3b53e3
commit 4e60cb3
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 51 deletions.
diff --git a/tutor/hooks/catalog.py b/tutor/hooks/catalog.py
@@ -514,8 +514,15 @@ def your_filter_callback(some_data):
     #: :param str file_path: The path to the file being checked.
     IS_FILE_RENDERED: Filter[bool, [str]] = Filter()
 
-    LMS_WORKER_COMMAND: Filter[dict[str, str], []] = Filter()
-    CMS_WORKER_COMMAND: Filter[dict[str, str], []] = Filter()
+    #: List of parameters to use when starting the LMS Celery worker ``celery worker ...``.
+    #:
+    #: :param list[str] command: the list of paramaters to use as the celery command.
+    LMS_WORKER_COMMAND: Filter[list[str], []] = Filter()
+
+    #: List of parameters to use when starting the CMS Celery worker ``celery worker ...``.
+    #:
+    #: :param list[str] command: the list of paramaters to use as the celery command.
+    CMS_WORKER_COMMAND: Filter[list[str], []] = Filter()
 
 
 class Contexts:

diff --git a/tutor/plugins/openedx.py b/tutor/plugins/openedx.py
@@ -146,62 +146,54 @@ def is_directory_mounted(image_name: str, dirname: str) -> bool:
 
 @hooks.Filters.LMS_WORKER_COMMAND.add(priority=hooks.priorities.HIGH)
 def _add_default_lms_worker_parameters(
-    worker_configs: dict[str, str]
-) -> dict[str, str]:
-    worker_configs = {
-        "loglevel": "info",
-        "hostname": "edx.lms.core.default.%%h",
-        "queues": "edx.lms.core.default,edx.lms.core.high,edx.lms.core.high_mem",
-        "max-tasks-per-child": "100",
-    }
-    return worker_configs
+    command: list[str]
+) -> list[str]:
+    command = [
+        "celery",
+        "worker",
+        "--app=lms.celery",
+        "--loglevel=info",
+        "--hostname=edx.lms.core.default.%%h",
+        "--queues=edx.lms.core.default,edx.lms.core.high,edx.lms.core.high_mem",
+        "--max-tasks-per-child=100",
+    ]
+    return command
 
 
 @hooks.Filters.CMS_WORKER_COMMAND.add(priority=hooks.priorities.HIGH)
 def _add_default_cms_worker_parameters(
-    worker_configs: dict[str, str]
-) -> dict[str, str]:
-    worker_configs = {
-        "loglevel": "info",
-        "hostname": "edx.cms.core.default.%%h",
-        "queues": "edx.cms.core.default,edx.cms.core.high,edx.cms.core.low",
-        "max-tasks-per-child": "100",
-    }
-    return worker_configs
-
-
-@hooks.lru_cache
-def get_cms_celery_parameters() -> dict[str, str]:
-    """
-    This function is cached for performance.
-    """
-    return hooks.Filters.CMS_WORKER_COMMAND.apply({})
+    command: list[str]
+) -> list[str]:
+    command = [
+        "celery",
+        "worker",
+        "--app=cms.celery",
+        "--loglevel=info",
+        "--hostname=edx.cms.core.default.%%h",
+        "--queues=edx.cms.core.default,edx.cms.core.high,edx.cms.core.low",
+        "--max-tasks-per-child=100",
+    ]
+    return command
 
 
 @hooks.lru_cache
-def get_lms_celery_parameters() -> dict[str, str]:
-    """
-    This function is cached for performance.
-    """
-    return hooks.Filters.LMS_WORKER_COMMAND.apply({})
-
-
-def iter_cms_celery_parameters() -> dict[str, str]:
+def iter_cms_celery_parameters() -> t.Iterator[str]:
     """
     Yield:
 
-        (name, dict)
+        str
     """
-    return {name: config for name, config in get_cms_celery_parameters().items()}
+    yield from hooks.Filters.CMS_WORKER_COMMAND.iterate()
 
 
-def iter_lms_celery_parameters() -> dict[str, str]:
+@hooks.lru_cache
+def iter_lms_celery_parameters() -> t.Iterator[str]:
     """
     Yield:
 
-        (name, dict)
+        str
     """
-    return {name: config for name, config in get_lms_celery_parameters().items()}
+    yield from hooks.Filters.LMS_WORKER_COMMAND.iterate()
 
 
 hooks.Filters.ENV_TEMPLATE_VARIABLES.add_items(

diff --git a/tutor/templates/k8s/deployments.yml b/tutor/templates/k8s/deployments.yml
@@ -142,10 +142,8 @@ spec:
         - name: cms-worker
           image: {{ DOCKER_IMAGE_OPENEDX }}
           args:
-          - "celery"
-          - "worker"
-          - "--app=cms.celery"{% for setting, value in iter_cms_celery_parameters().items() %}
-          - "--{{setting}}={{value}}"{% endfor %}
+          {% for value in iter_cms_celery_parameters() %}
+          - "{{value}}"{% endfor %}
           env:
           - name: SERVICE_VARIANT
             value: cms
@@ -255,10 +253,8 @@ spec:
         - name: lms-worker
           image: {{ DOCKER_IMAGE_OPENEDX }}
           args:
-          - "celery"
-          - "worker"
-          - "--app=lms.celery"{% for setting, value in iter_lms_celery_parameters().items() %}
-          - "--{{setting}}={{value}}"{% endfor %}
+          {% for value in iter_lms_celery_parameters() %}
+          - "{{value}}"{% endfor %}
           env:
           - name: SERVICE_VARIANT
             value: lms

diff --git a/tutor/templates/local/docker-compose.prod.yml b/tutor/templates/local/docker-compose.prod.yml
@@ -31,7 +31,7 @@ services:
     environment:
       SERVICE_VARIANT: lms
       DJANGO_SETTINGS_MODULE: lms.envs.tutor.production
-    command: celery worker --app=lms.celery{% for setting, value in iter_lms_celery_parameters().items() %} "--{{setting}}={{value}}"{% endfor %}
+    command:{% for value in iter_lms_celery_parameters() %} {{value}}{% endfor %}
     restart: unless-stopped
     volumes:
       - ../apps/openedx/settings/lms:/openedx/edx-platform/lms/envs/tutor:ro
@@ -50,7 +50,7 @@ services:
     environment:
       SERVICE_VARIANT: cms
       DJANGO_SETTINGS_MODULE: cms.envs.tutor.production
-    command: celery worker --app=cms.celery{% for setting, value in iter_cms_celery_parameters().items() %} "--{{setting}}={{value}}"{% endfor %}
+    command:{% for value in iter_cms_celery_parameters() %} {{value}}{% endfor %}
     restart: unless-stopped
     volumes:
       - ../apps/openedx/settings/lms:/openedx/edx-platform/lms/envs/tutor:ro