From 630ffd7e68827d5f562f2f847254bea29c21f3f4 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Mon, 14 Nov 2022 10:52:51 +0000 Subject: [PATCH 1/3] Correct overlooked argument name This changed in a401c57. --- django_lightweight_queue/management/commands/queue_runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django_lightweight_queue/management/commands/queue_runner.py b/django_lightweight_queue/management/commands/queue_runner.py index 6e4fa06..90181c0 100644 --- a/django_lightweight_queue/management/commands/queue_runner.py +++ b/django_lightweight_queue/management/commands/queue_runner.py @@ -95,7 +95,7 @@ def validate_and_normalise(self, options: Dict[str, Any]) -> None: if options['exact_configuration']: if not options['extra_settings']: raise CommandError( - "Must provide a value for '--config' when using " + "Must provide a value for '--extra-settings' when using " "'--exact-configuration'.", ) From 65a99b41134d7f0694d9e1f56dffbe73ef8e0da8 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Mon, 14 Nov 2022 10:54:12 +0000 Subject: [PATCH 2/3] Extract base class for handling `--extra-settings` --- django_lightweight_queue/command_utils.py | 67 +++++++++++++++++++ .../commands/queue_configuration.py | 40 ++--------- .../management/commands/queue_runner.py | 60 ++++------------- 3 files changed, 85 insertions(+), 82 deletions(-) create mode 100644 django_lightweight_queue/command_utils.py diff --git a/django_lightweight_queue/command_utils.py b/django_lightweight_queue/command_utils.py new file mode 100644 index 0000000..bd995c4 --- /dev/null +++ b/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 diff --git a/django_lightweight_queue/management/commands/queue_configuration.py b/django_lightweight_queue/management/commands/queue_configuration.py index 822cdf8..096bd57 100644 --- a/django_lightweight_queue/management/commands/queue_configuration.py +++ b/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("========================") diff --git a/django_lightweight_queue/management/commands/queue_runner.py b/django_lightweight_queue/management/commands/queue_runner.py index 90181c0..c9e0300 100644 --- a/django_lightweight_queue/management/commands/queue_runner.py +++ b/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,17 +59,9 @@ 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 '--extra-settings' when using " "'--exact-configuration'.", @@ -127,7 +95,12 @@ 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: @@ -135,11 +108,6 @@ def touch_filename(name: str) -> Optional[str]: 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 From 5230a447f6e80d4b90cfbb038a613f5cc5c602f9 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Mon, 14 Nov 2022 11:06:04 +0000 Subject: [PATCH 3/3] Pass through extra settings to the worker process Fixes https://github.com/thread/django-lightweight-queue/issues/78 --- .../management/commands/queue_runner.py | 2 +- .../management/commands/queue_worker.py | 9 +++++++-- django_lightweight_queue/runner.py | 7 +++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/django_lightweight_queue/management/commands/queue_runner.py b/django_lightweight_queue/management/commands/queue_runner.py index c9e0300..e01c901 100644 --- a/django_lightweight_queue/management/commands/queue_runner.py +++ b/django_lightweight_queue/management/commands/queue_runner.py @@ -132,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. diff --git a/django_lightweight_queue/management/commands/queue_worker.py b/django_lightweight_queue/management/commands/queue_worker.py index 3dd4a4d..ada0d43 100644 --- a/django_lightweight_queue/management/commands/queue_worker.py +++ b/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, diff --git a/django_lightweight_queue/runner.py b/django_lightweight_queue/runner.py index b193066..174d50f 100644 --- a/django_lightweight_queue/runner.py +++ b/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)