Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions django_lightweight_queue/command_utils.py
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered just making this part of handle on the super class and then having subclasses call super().handle(...) instead of a custom method?

I can't see any weird side effects and it's probably a bit more expected when going through inheritance.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did, unfortunately Django's BaseCommand.handle raises NotImplementedError so we'd need where to stop the super() calls to avoid errors and also means that calling super().handle(...) in an implementation could easily look like it's a bug.
I also considered an approach of adding this functionality as a decorator (which would make it harder to get wrong) however that felt more complicated (class decorators being more complicated than function ones) and likely wouldn't have a great way to support the return value.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right gotcha.


print("django-lightweight-queue")
print("========================")
Expand Down
64 changes: 16 additions & 48 deletions django_lightweight_queue/management/commands/queue_runner.py
Original file line number Diff line number Diff line change
@@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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'.",
)

Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
9 changes: 7 additions & 2 deletions django_lightweight_queue/management/commands/queue_worker.py
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -40,6 +43,8 @@ def handle(
touch_filename: str,
**options: Any
) -> None:
super().handle_extra_settings(**options)

worker = Worker(
queue=queue,
worker_num=number,
Expand Down
7 changes: 7 additions & 0 deletions django_lightweight_queue/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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)

Expand Down