Merge branch 'worker-passthrough-extra-settings'

Author: PeterJCLaw

django_lightweight_queue/command_utils.py

@@ -0,0 +1,67 @@
+import warnings
+from typing import Any, Optional
+
+from django.core.management.base import BaseCommand, CommandParser
+
+from .utils import load_extra_settings
+from .constants import SETTING_NAME_PREFIX
+
+
+class CommandWithExtraSettings(BaseCommand):
+    """
+    Base class for handling `--extra-settings`.
+
+    Derived classes must call `handle_extra_settings` at the top of their
+    `handle` method. For example:
+
+        class Command(CommandWithExtraSettings):
+            def handle(self, **options: Any) -> None:
+                super().handle_extra_settings(**options)
+                ...
+    """
+
+    def add_arguments(self, parser: CommandParser) -> None:
+        super().add_arguments(parser)
+
+        extra_settings_group = parser.add_mutually_exclusive_group()
+        extra_settings_group.add_argument(
+            '--config',
+            action='store',
+            default=None,
+            help="The path to an additional django-style config file to load "
+                 "(this spelling is deprecated in favour of '--extra-settings')",
+        )
+        extra_settings_group.add_argument(
+            '--extra-settings',
+            action='store',
+            default=None,
+            help="The path to an additional django-style settings file to load. "
+                 f"{SETTING_NAME_PREFIX}* settings discovered in this file will "
+                 "override those from the default Django settings.",
+        )
+
+    def handle_extra_settings(
+        self,
+        *,
+        config: Optional[str] = None,
+        extra_settings: Optional[str],
+        **_: Any
+    ) -> Optional[str]:
+        """
+        Load extra settings if there are any.
+
+        Returns the filename (if any) of the extra settings that have been loaded.
+        """
+
+        if config is not None:
+            warnings.warn(
+                "Use of '--config' is deprecated in favour of '--extra-settings'.",
+                category=DeprecationWarning,
+            )
+            extra_settings = config
+
+        # Configuration overrides
+        if extra_settings is not None:
+            load_extra_settings(extra_settings)
+
+        return extra_settings

django_lightweight_queue/management/commands/queue_configuration.py

@@ -1,46 +1,14 @@
-import warnings
 from typing import Any
 
-from django.core.management.base import BaseCommand, CommandParser
-
 from ... import app_settings
-from ...utils import get_backend, get_queue_counts, load_extra_settings
-from ...constants import SETTING_NAME_PREFIX
+from ...utils import get_backend, get_queue_counts
+from ...command_utils import CommandWithExtraSettings
 from ...cron_scheduler import get_cron_config
 
 
-class Command(BaseCommand):
-    def add_arguments(self, parser: CommandParser) -> None:
-        extra_settings_group = parser.add_mutually_exclusive_group()
-        extra_settings_group.add_argument(
-            '--config',
-            action='store',
-            default=None,
-            help="The path to an additional django-style config file to load "
-                 "(this spelling is deprecated in favour of '--extra-settings')",
-        )
-        extra_settings_group.add_argument(
-            '--extra-settings',
-            action='store',
-            default=None,
-            help="The path to an additional django-style settings file to load. "
-                 f"{SETTING_NAME_PREFIX}* settings discovered in this file will "
-                 "override those from the default Django settings.",
-        )
-
+class Command(CommandWithExtraSettings):
     def handle(self, **options: Any) -> None:
-        extra_config = options.pop('config')
-        if extra_config is not None:
-            warnings.warn(
-                "Use of '--config' is deprecated in favour of '--extra-settings'.",
-                category=DeprecationWarning,
-            )
-            options['extra_settings'] = extra_config
-
-        # Configuration overrides
-        extra_settings = options['extra_settings']
-        if extra_settings is not None:
-            load_extra_settings(extra_settings)
+        super().handle_extra_settings(**options)
 
         print("django-lightweight-queue")
         print("========================")

django_lightweight_queue/management/commands/queue_runner.py

@@ -1,29 +1,21 @@
-import warnings
 from typing import Any, Dict, Optional
 
 import daemonize
 
 from django.apps import apps
-from django.core.management.base import (
-    BaseCommand,
-    CommandError,
-    CommandParser,
-)
+from django.core.management.base import CommandError, CommandParser
 
 from ...types import QueueName
-from ...utils import (
-    get_logger,
-    get_backend,
-    get_middleware,
-    load_extra_settings,
-)
+from ...utils import get_logger, get_backend, get_middleware
 from ...runner import runner
-from ...constants import SETTING_NAME_PREFIX
+from ...command_utils import CommandWithExtraSettings
 from ...machine_types import Machine, PooledMachine, DirectlyConfiguredMachine
 
 
-class Command(BaseCommand):
+class Command(CommandWithExtraSettings):
     def add_arguments(self, parser: CommandParser) -> None:
+        super().add_arguments(parser)
+
         parser.add_argument(
             '--pidfile',
             action='store',
@@ -58,22 +50,6 @@ def add_arguments(self, parser: CommandParser) -> None:
             default=None,
             help="Only run the given queue, useful for local debugging",
         )
-        extra_settings_group = parser.add_mutually_exclusive_group()
-        extra_settings_group.add_argument(
-            '--config',
-            action='store',
-            default=None,
-            help="The path to an additional django-style config file to load "
-                 "(this spelling is deprecated in favour of '--extra-settings')",
-        )
-        extra_settings_group.add_argument(
-            '--extra-settings',
-            action='store',
-            default=None,
-            help="The path to an additional django-style settings file to load. "
-                 f"{SETTING_NAME_PREFIX}* settings discovered in this file will "
-                 "override those from the default Django settings.",
-        )
         parser.add_argument(
             '--exact-configuration',
             action='store_true',
@@ -83,19 +59,11 @@ def add_arguments(self, parser: CommandParser) -> None:
                  " '--of'.",
         )
 
-    def validate_and_normalise(self, options: Dict[str, Any]) -> None:
-        extra_config = options.pop('config')
-        if extra_config is not None:
-            warnings.warn(
-                "Use of '--config' is deprecated in favour of '--extra-settings'.",
-                category=DeprecationWarning,
-            )
-            options['extra_settings'] = extra_config
-
+    def validate_and_normalise(self, options: Dict[str, Any], had_extra_settings: bool) -> None:
         if options['exact_configuration']:
-            if not options['extra_settings']:
+            if not had_extra_settings:
                 raise CommandError(
-                    "Must provide a value for '--config' when using "
+                    "Must provide a value for '--extra-settings' when using "
                     "'--exact-configuration'.",
                 )
 
@@ -127,19 +95,19 @@ def validate_and_normalise(self, options: Dict[str, Any]) -> None:
     def handle(self, **options: Any) -> None:
         logger = get_logger('dlq.master')
 
-        self.validate_and_normalise(options)
+        extra_settings = super().handle_extra_settings(**options)
+
+        self.validate_and_normalise(
+            options,
+            had_extra_settings=extra_settings is not None,
+        )
 
         def touch_filename(name: str) -> Optional[str]:
             try:
                 return options['touchfile'] % name
             except TypeError:
                 return None
 
-        # Configuration overrides
-        extra_config = options['extra_settings']
-        if extra_config is not None:
-            load_extra_settings(extra_config)
-
         logger.info("Starting queue master")
 
         # Ensure children will be able to import our backend
@@ -164,7 +132,7 @@ def touch_filename(name: str) -> Optional[str]:
             )
 
         def run() -> None:
-            runner(touch_filename, machine, logger)
+            runner(touch_filename, machine, logger, extra_settings)
 
         # fork() only after we have started enough to catch failure, including
         # being able to write to our pidfile.

django_lightweight_queue/management/commands/queue_worker.py

@@ -1,15 +1,18 @@
 from typing import Any
 
-from django.core.management.base import BaseCommand, CommandParser
+from django.core.management.base import CommandParser
 
 from ...types import QueueName, WorkerNumber
 from ...worker import Worker
+from ...command_utils import CommandWithExtraSettings
 
 
-class Command(BaseCommand):
+class Command(CommandWithExtraSettings):
     help = "Run an individual queue worker"  # noqa:A003 # inherited name
 
     def add_arguments(self, parser: CommandParser) -> None:
+        super().add_arguments(parser)
+
         parser.add_argument(
             'queue',
             help="queue for which this is a worker",
@@ -40,6 +43,8 @@ def handle(
         touch_filename: str,
         **options: Any
     ) -> None:
+        super().handle_extra_settings(**options)
+
         worker = Worker(
             queue=queue,
             worker_num=number,

django_lightweight_queue/runner.py

@@ -20,6 +20,7 @@ def runner(
     touch_filename_fn: Callable[[QueueName], Optional[str]],
     machine: Machine,
     logger: Logger,
+    extra_settings_filename: Optional[str],
 ) -> None:
     set_process_title("Master process")
 
@@ -117,6 +118,12 @@ def handle_term(signum: int, stack: object) -> None:
                         touch_filename,
                     ])
 
+                if extra_settings_filename is not None:
+                    args.extend([
+                        '--extra-settings',
+                        extra_settings_filename,
+                    ])
+
                 worker = subprocess.Popen(args)
                 workers[(queue, worker_num)] = (worker, worker_name)