From f40154f174a5632058c52b27ff1212887e861001 Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Sat, 18 Jun 2022 18:27:54 -0400 Subject: [PATCH 01/21] resolve conflicts --- README.md | 25 +++- django_lightweight_queue/app_settings.py | 111 ++++++++++++------ django_lightweight_queue/backends/redis.py | 12 +- .../backends/reliable_redis.py | 12 +- django_lightweight_queue/exposition.py | 6 +- .../commands/queue_configuration.py | 4 +- django_lightweight_queue/runner.py | 6 +- django_lightweight_queue/task.py | 4 +- django_lightweight_queue/utils.py | 23 ++-- django_lightweight_queue/worker.py | 8 +- pyproject.toml | 2 +- tests/test_pause_resume.py | 2 +- tests/test_reliable_redis_backend.py | 4 +- tests/test_task.py | 4 +- 14 files changed, 139 insertions(+), 84 deletions(-) diff --git a/README.md b/README.md index abce973..68ecac1 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,19 @@ backends are great candidates for community contributions. ## Basic Usage +Start by adding `django_lightweight_queue` to your `INSTALLED_APPS`: + +```python +INSTALLED_APPS = [ + "django.contrib.admin", + "django.contrib.auth", + ..., + "django_lightweight_queue", +] +``` + +After that, define your task in any file you want: + ```python import time from django_lightweight_queue import task @@ -67,12 +80,12 @@ present in the specified file are inherited from the global configuration. There are four built-in backends: -| Backend | Type | Description | -| -------------- | ----------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Synchronous | Development | Executes the task inline, without any actual queuing. | -| Redis | Production | Executes tasks at-most-once using [Redis][redis] for storage of the enqueued tasks. | -| Reliable Redis | Production | Executes tasks at-least-once using [Redis][redis] for storage of the enqueued tasks (subject to Redis consistency). Does not guarantee the task _completes_. | -| Debug Web | Debugging | Instead of running jobs it prints the url to a view that can be used to run a task in a transaction which will be rolled back. This is useful for debugging and optimising tasks. | +| Backend | Import Location | Type | Description | +| -------------- |:----------------------------------------------------------------------| ----------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Synchronous | django_lightweight_queue.backends.synchronous.SynchronousBackend | Development | Executes the task inline, without any actual queuing. | +| Redis | django_lightweight_queue.backends.redis.RedisBackend | Production | Executes tasks at-most-once using [Redis][redis] for storage of the enqueued tasks. | +| Reliable Redis | django_lightweight_queue.backends.reliable_redis.ReliableRedisBackend | Production | Executes tasks at-least-once using [Redis][redis] for storage of the enqueued tasks (subject to Redis consistency). Does not guarantee the task _completes_. | +| Debug Web | django_lightweight_queue.backends.debug_web.DebugWebBackend | Debugging | Instead of running jobs it prints the url to a view that can be used to run a task in a transaction which will be rolled back. This is useful for debugging and optimising tasks. | [redis]: https://redis.io/ diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index b4bd19c..24ae5e0 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -1,6 +1,6 @@ -from typing import Dict, Union, Mapping, TypeVar, Callable, Optional, Sequence +from typing import Union, Mapping, TypeVar, Callable, Optional, Sequence -from django.conf import settings +from django.conf import settings as django_settings from . import constants from .types import Logger, QueueName @@ -8,45 +8,86 @@ T = TypeVar('T') -def setting(suffix: str, default: T) -> T: - attr_name = '{}{}'.format(constants.SETTING_NAME_PREFIX, suffix) - return getattr(settings, attr_name, default) +class Settings(): + def _get(self, suffix: str, default: T) -> T: + attr_name = '{}{}'.format(constants.SETTING_NAME_PREFIX, suffix) + return getattr(django_settings, attr_name, default) + # adjustable values at runtime + _backend = None -WORKERS = setting('WORKERS', {}) # type: Dict[QueueName, int] -BACKEND = setting( - 'BACKEND', - 'django_lightweight_queue.backends.synchronous.SynchronousBackend', -) # type: str + @property + def WORKERS(self): + return self._get('WORKERS', {}) -LOGGER_FACTORY = setting( - 'LOGGER_FACTORY', - 'logging.getLogger', -) # type: Union[str, Callable[[str], Logger]] + @property + def BACKEND(self): + if not self._backend: + self._backend = self._get( + 'BACKEND', + 'django_lightweight_queue.backends.synchronous.SynchronousBackend', + ) + return self._backend # type: str -# Allow per-queue overrides of the backend. -BACKEND_OVERRIDES = setting('BACKEND_OVERRIDES', {}) # type: Mapping[QueueName, str] + @BACKEND.setter + def BACKEND(self, value): + self._backend = value -MIDDLEWARE = setting('MIDDLEWARE', ( - 'django_lightweight_queue.middleware.logging.LoggingMiddleware', - 'django_lightweight_queue.middleware.transaction.TransactionMiddleware', -)) # type: Sequence[str] + @property + def LOGGER_FACTORY(self): + return self._get( + 'LOGGER_FACTORY', + 'logging.getLogger', + ) # type: Union[str, Callable[[str], Logger]] -# Apps to ignore when looking for tasks. Apps must be specified as the dotted -# name used in `INSTALLED_APPS`. This is expected to be useful when you need to -# have a file called `tasks.py` within an app, but don't want -# django-lightweight-queue to import that file. -# Note: this _doesn't_ prevent tasks being registered from these apps. -IGNORE_APPS = setting('IGNORE_APPS', ()) # type: Sequence[str] + @property + def BACKEND_OVERRIDES(self): + # Allow per-queue overrides of the backend. + return self._get('BACKEND_OVERRIDES', {}) # type: Mapping[QueueName, str] -# Backend-specific settings -REDIS_HOST = setting('REDIS_HOST', '127.0.0.1') # type: str -REDIS_PORT = setting('REDIS_PORT', 6379) # type: int -REDIS_PASSWORD = setting('REDIS_PASSWORD', None) # type: Optional[str] -REDIS_PREFIX = setting('REDIS_PREFIX', '') # type: str + @property + def MIDDLEWARE(self): + return self._get('MIDDLEWARE', ( + 'django_lightweight_queue.middleware.logging.LoggingMiddleware', + )) # type: Sequence[str] -ENABLE_PROMETHEUS = setting('ENABLE_PROMETHEUS', False) # type: bool -# Workers will export metrics on this port, and ports following it -PROMETHEUS_START_PORT = setting('PROMETHEUS_START_PORT', 9300) # type: int + @property + def IGNORE_APPS(self): + # Apps to ignore when looking for tasks. Apps must be specified as the dotted + # name used in `INSTALLED_APPS`. This is expected to be useful when you need to + # have a file called `tasks.py` within an app, but don't want + # django-lightweight-queue to import that file. + # Note: this _doesn't_ prevent tasks being registered from these apps. + return self._get('IGNORE_APPS', ()) # type: Sequence[str] -ATOMIC_JOBS = setting('ATOMIC_JOBS', True) # type: bool + @property + def REDIS_HOST(self): + return self._get('REDIS_HOST', '127.0.0.1') # type: str + + @property + def REDIS_PORT(self): + return self._get('REDIS_PORT', 6379) # type: int + + @property + def REDIS_PASSWORD(self): + return self._get('REDIS_PASSWORD', None) # type: Optional[str] + + @property + def REDIS_PREFIX(self): + return self._get('REDIS_PREFIX', '') # type: str + + @property + def ENABLE_PROMETHEUS(self): + return self._get('ENABLE_PROMETHEUS', False) # type: bool + + @property + def PROMETHEUS_START_PORT(self): + # Workers will export metrics on this port, and ports following it + return self._get('PROMETHEUS_START_PORT', 9300) # type: int + + @property + def ATOMIC_JOBS(self): + return self._get('ATOMIC_JOBS', True) # type: bool + + +settings = Settings() diff --git a/django_lightweight_queue/backends/redis.py b/django_lightweight_queue/backends/redis.py index 6d4b689..72b5767 100644 --- a/django_lightweight_queue/backends/redis.py +++ b/django_lightweight_queue/backends/redis.py @@ -3,11 +3,11 @@ import redis -from .. import app_settings from ..job import Job from .base import BackendWithPauseResume from ..types import QueueName, WorkerNumber from ..utils import block_for_time +from ..app_settings import settings class RedisBackend(BackendWithPauseResume): @@ -17,9 +17,9 @@ class RedisBackend(BackendWithPauseResume): def __init__(self) -> None: self.client = redis.StrictRedis( - host=app_settings.REDIS_HOST, - port=app_settings.REDIS_PORT, - password=app_settings.REDIS_PASSWORD, + host=settings.REDIS_HOST, + port=settings.REDIS_PORT, + password=settings.REDIS_PASSWORD, ) def enqueue(self, job: Job, queue: QueueName) -> None: @@ -79,9 +79,9 @@ def is_paused(self, queue: QueueName) -> bool: return bool(self.client.exists(self._pause_key(queue))) def _key(self, queue: QueueName) -> str: - if app_settings.REDIS_PREFIX: + if settings.REDIS_PREFIX: return '{}:django_lightweight_queue:{}'.format( - app_settings.REDIS_PREFIX, + settings.REDIS_PREFIX, queue, ) diff --git a/django_lightweight_queue/backends/reliable_redis.py b/django_lightweight_queue/backends/reliable_redis.py index 8012763..b01b5cc 100644 --- a/django_lightweight_queue/backends/reliable_redis.py +++ b/django_lightweight_queue/backends/reliable_redis.py @@ -3,11 +3,11 @@ import redis -from .. import app_settings from ..job import Job from .base import BackendWithDeduplicate, BackendWithPauseResume from ..types import QueueName, WorkerNumber from ..utils import block_for_time, get_worker_numbers +from ..app_settings import settings from ..progress_logger import ProgressLogger, NULL_PROGRESS_LOGGER # Work around https://github.com/python/mypy/issues/9914. Name needs to match @@ -39,9 +39,9 @@ class ReliableRedisBackend(BackendWithDeduplicate, BackendWithPauseResume): def __init__(self) -> None: self.client = redis.StrictRedis( - host=app_settings.REDIS_HOST, - port=app_settings.REDIS_PORT, - password=app_settings.REDIS_PASSWORD, + host=settings.REDIS_HOST, + port=settings.REDIS_PORT, + password=settings.REDIS_PASSWORD, ) def startup(self, queue: QueueName) -> None: @@ -245,9 +245,9 @@ def _processing_key(self, queue: QueueName, worker_number: WorkerNumber) -> str: return self._prefix_key(key) def _prefix_key(self, key: str) -> str: - if app_settings.REDIS_PREFIX: + if settings.REDIS_PREFIX: return '{}:{}'.format( - app_settings.REDIS_PREFIX, + settings.REDIS_PREFIX, key, ) diff --git a/django_lightweight_queue/exposition.py b/django_lightweight_queue/exposition.py index d17f7ab..9ebba15 100644 --- a/django_lightweight_queue/exposition.py +++ b/django_lightweight_queue/exposition.py @@ -6,8 +6,8 @@ from prometheus_client.exposition import MetricsHandler -from . import app_settings from .types import QueueName, WorkerNumber +from .app_settings import settings def get_config_response( @@ -23,7 +23,7 @@ def get_config_response( "targets": [ "{}:{}".format( gethostname(), - app_settings.PROMETHEUS_START_PORT + index, + settings.PROMETHEUS_START_PORT + index, ), ], "labels": { @@ -60,7 +60,7 @@ def __init__(self, *args, **kwargs): super(MetricsServer, self).__init__(*args, **kwargs) def run(self): - httpd = HTTPServer(('0.0.0.0', app_settings.PROMETHEUS_START_PORT), RequestHandler) + httpd = HTTPServer(('0.0.0.0', settings.PROMETHEUS_START_PORT), RequestHandler) httpd.timeout = 2 try: diff --git a/django_lightweight_queue/management/commands/queue_configuration.py b/django_lightweight_queue/management/commands/queue_configuration.py index feb6f4c..7c4aaf1 100644 --- a/django_lightweight_queue/management/commands/queue_configuration.py +++ b/django_lightweight_queue/management/commands/queue_configuration.py @@ -2,8 +2,8 @@ from django.core.management.base import BaseCommand, CommandParser -from ... import app_settings from ...utils import get_backend, get_queue_counts, load_extra_config +from ...app_settings import settings from ...cron_scheduler import get_cron_config @@ -37,7 +37,7 @@ def handle(self, **options: Any) -> None: print("") print("Middleware:") - for x in app_settings.MIDDLEWARE: + for x in settings.MIDDLEWARE: print(" * {}".format(x)) print("") diff --git a/django_lightweight_queue/runner.py b/django_lightweight_queue/runner.py index f3811e4..712c66f 100644 --- a/django_lightweight_queue/runner.py +++ b/django_lightweight_queue/runner.py @@ -4,10 +4,10 @@ import subprocess from typing import Dict, Tuple, Callable, Optional -from . import app_settings from .types import Logger, QueueName, WorkerNumber from .utils import get_backend, set_process_title from .exposition import metrics_http_server +from .app_settings import settings from .machine_types import Machine from .cron_scheduler import ( CronScheduler, @@ -64,7 +64,7 @@ def handle_term(signum: int, stack: object) -> None: for x in machine.worker_names } # type: Dict[Tuple[QueueName, WorkerNumber], Tuple[Optional[subprocess.Popen[bytes]], str]] - if app_settings.ENABLE_PROMETHEUS: + if settings.ENABLE_PROMETHEUS: metrics_server = metrics_http_server(machine.worker_names) metrics_server.start() @@ -107,7 +107,7 @@ def handle_term(signum: int, stack: object) -> None: queue, str(worker_num), '--prometheus-port', - str(app_settings.PROMETHEUS_START_PORT + index), + str(settings.PROMETHEUS_START_PORT + index), ] touch_filename = touch_filename_fn(queue) diff --git a/django_lightweight_queue/task.py b/django_lightweight_queue/task.py index 1e457c8..2da421f 100644 --- a/django_lightweight_queue/task.py +++ b/django_lightweight_queue/task.py @@ -12,10 +12,10 @@ Optional, ) -from . import app_settings from .job import Job from .types import QueueName from .utils import get_backend, contribute_implied_queue_name +from .app_settings import settings TCallable = TypeVar('TCallable', bound=Callable[..., Any]) @@ -82,7 +82,7 @@ def slow_fn(arg): """ if atomic is None: - atomic = app_settings.ATOMIC_JOBS + atomic = settings.ATOMIC_JOBS self.queue = QueueName(queue) self.timeout = timeout diff --git a/django_lightweight_queue/utils.py b/django_lightweight_queue/utils.py index b119e8c..ba712d8 100644 --- a/django_lightweight_queue/utils.py +++ b/django_lightweight_queue/utils.py @@ -21,8 +21,9 @@ from django.core.exceptions import MiddlewareNotUsed from django.utils.module_loading import module_has_submodule -from . import constants, app_settings +from . import constants from .types import Logger, QueueName, WorkerNumber +from .app_settings import settings if TYPE_CHECKING: from .backends.base import BaseBackend @@ -47,7 +48,7 @@ def with_prefix(names: Iterable[str]) -> Set[str]: for name in names ) - setting_names = get_setting_names(app_settings) + setting_names = get_setting_names(settings) extra_names = get_setting_names(extra_settings) unexpected_names = extra_names - with_prefix(setting_names) @@ -58,7 +59,7 @@ def with_prefix(names: Iterable[str]) -> Set[str]: override_names = extra_names - unexpected_names for name in override_names: short_name = name[len(constants.SETTING_NAME_PREFIX):] - setattr(app_settings, short_name, getattr(extra_settings, name)) + setattr(settings, short_name, getattr(extra_settings, name)) @lru_cache() @@ -72,19 +73,19 @@ def get_path(path: str) -> Any: @lru_cache() def get_backend(queue: QueueName) -> 'BaseBackend': - return get_path(app_settings.BACKEND_OVERRIDES.get( + return get_path(settings.BACKEND_OVERRIDES.get( queue, - app_settings.BACKEND, + settings.BACKEND, ))() @lru_cache() def get_logger(name: str) -> Logger: - get_logger_fn = app_settings.LOGGER_FACTORY + get_logger_fn = settings.LOGGER_FACTORY if not callable(get_logger_fn): get_logger_fn = cast( Callable[[str], Logger], - get_path(app_settings.LOGGER_FACTORY), + get_path(settings.LOGGER_FACTORY), ) return get_logger_fn(name) @@ -93,7 +94,7 @@ def get_logger(name: str) -> Logger: def get_middleware() -> List[Any]: middleware = [] - for path in app_settings.MIDDLEWARE: + for path in settings.MIDDLEWARE: try: middleware.append(get_path(path)()) except MiddlewareNotUsed: @@ -113,12 +114,12 @@ def contribute_implied_queue_name(queue: QueueName) -> None: "Queues have already been enumerated, ensure that " "'contribute_implied_queue_name' is called during setup.", ) - app_settings.WORKERS.setdefault(queue, 1) + settings.WORKERS.setdefault(queue, 1) def get_queue_counts() -> Mapping[QueueName, int]: refuse_further_implied_queues() - return app_settings.WORKERS + return settings.WORKERS def get_worker_numbers(queue: QueueName) -> Collection[WorkerNumber]: @@ -143,7 +144,7 @@ def import_all_submodules(name: str, exclude: Sequence[str] = ()) -> None: def load_all_tasks() -> None: - import_all_submodules('tasks', app_settings.IGNORE_APPS) + import_all_submodules('tasks', settings.IGNORE_APPS) def block_for_time( diff --git a/django_lightweight_queue/worker.py b/django_lightweight_queue/worker.py index da25dd2..63b9ee7 100644 --- a/django_lightweight_queue/worker.py +++ b/django_lightweight_queue/worker.py @@ -12,12 +12,12 @@ from django.db import connections, transaction -from . import app_settings from .types import QueueName, WorkerNumber from .utils import get_logger, get_backend, set_process_title +from .app_settings import settings from .backends.base import BaseBackend -if app_settings.ENABLE_PROMETHEUS: +if settings.ENABLE_PROMETHEUS: job_duration = Summary( 'item_processed_seconds', "Item processing time", @@ -54,7 +54,7 @@ def __init__( self.name = '{}/{}'.format(queue, worker_num) def run(self) -> None: - if app_settings.ENABLE_PROMETHEUS and self.prometheus_port is not None: + if settings.ENABLE_PROMETHEUS and self.prometheus_port is not None: self.log(logging.INFO, "Exporting metrics on port {}".format(self.prometheus_port)) start_http_server(self.prometheus_port) @@ -88,7 +88,7 @@ def run(self) -> None: item_processed = self.process(backend) post_process_time = time.time() - if app_settings.ENABLE_PROMETHEUS: + if settings.ENABLE_PROMETHEUS: job_duration.labels(self.queue).observe( post_process_time - pre_process_time, ) diff --git a/pyproject.toml b/pyproject.toml index 0c3996d..f78737d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "django-lightweight-queue" -version = "4.5.1" +version = "4.6.0" description = "Lightweight & modular queue and cron system for Django" authors = ["Thread Engineering "] license = "BSD-3-Clause" diff --git a/tests/test_pause_resume.py b/tests/test_pause_resume.py index eb70d38..842bb48 100644 --- a/tests/test_pause_resume.py +++ b/tests/test_pause_resume.py @@ -52,7 +52,7 @@ def setUp(self) -> None: # Can't use override_settings due to the copying of the settings values into # module values at startup. @mock.patch( - 'django_lightweight_queue.app_settings.BACKEND', + 'django_lightweight_queue.app_settings.Settings.BACKEND', new='django_lightweight_queue.backends.redis.RedisBackend', ) def test_pause_resume(self) -> None: diff --git a/tests/test_reliable_redis_backend.py b/tests/test_reliable_redis_backend.py index 6359ca3..9d5ad71 100644 --- a/tests/test_reliable_redis_backend.py +++ b/tests/test_reliable_redis_backend.py @@ -49,8 +49,8 @@ def mock_workers(self, workers: Mapping[str, int]) -> Iterator[None]: with unittest.mock.patch( 'django_lightweight_queue.utils._accepting_implied_queues', new=False, - ), unittest.mock.patch.dict( - 'django_lightweight_queue.app_settings.WORKERS', + ), unittest.mock.patch( + 'django_lightweight_queue.app_settings.Settings.WORKERS', workers, ): yield diff --git a/tests/test_task.py b/tests/test_task.py index 48f749b..0fe8a29 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -30,7 +30,7 @@ def mock_workers(self, workers: Mapping[str, int]) -> Iterator[None]: 'django_lightweight_queue.utils._accepting_implied_queues', new=False, ), unittest.mock.patch.dict( - 'django_lightweight_queue.app_settings.WORKERS', + 'django_lightweight_queue.app_settings.Settings.WORKERS', workers, ): yield @@ -54,7 +54,7 @@ def mocked_get_path(path: str) -> Any: return get_path(path) patch = mock.patch( - 'django_lightweight_queue.app_settings.BACKEND', + 'django_lightweight_queue.app_settings.Settings.BACKEND', new='test-backend', ) patch.start() From 7cce4c413292edfb4b3966dabb9eb9247f161f2f Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Sat, 18 Jun 2022 18:44:04 -0400 Subject: [PATCH 02/21] make all settings changeable at runtime --- django_lightweight_queue/app_settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index 24ae5e0..15132a0 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -15,6 +15,7 @@ def _get(self, suffix: str, default: T) -> T: # adjustable values at runtime _backend = None + _redis_password = None @property def WORKERS(self): From 548cb6e51df81e697defd39e33dc0e0e5e478e1d Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Sat, 18 Jun 2022 18:44:56 -0400 Subject: [PATCH 03/21] make all settings changeable at runtime, try 2 --- django_lightweight_queue/app_settings.py | 117 +++++++++++++++++++---- 1 file changed, 100 insertions(+), 17 deletions(-) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index 15132a0..3cf40a5 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -14,12 +14,29 @@ def _get(self, suffix: str, default: T) -> T: return getattr(django_settings, attr_name, default) # adjustable values at runtime + _workers = None _backend = None + _logger_factory = None + _backend_overrides = None + _middleware = None + _ignore_apps = None + _redis_host = None + _redis_port = None _redis_password = None + _redis_prefix = None + _enable_prometheus = None + _prometheus_start_port = None + _atomic_jobs = None @property def WORKERS(self): - return self._get('WORKERS', {}) + if not self._workers: + self._workers = self._get('WORKERS', {}) + return self._workers + + @WORKERS.setter + def WORKERS(self, value): + self._workers = value @property def BACKEND(self): @@ -36,21 +53,39 @@ def BACKEND(self, value): @property def LOGGER_FACTORY(self): - return self._get( - 'LOGGER_FACTORY', - 'logging.getLogger', - ) # type: Union[str, Callable[[str], Logger]] + if not self._logger_factory: + self._logger_factory = self._get( + 'LOGGER_FACTORY', + 'logging.getLogger', + ) + return self._logger_factory # type: Union[str, Callable[[str], Logger]] + + @LOGGER_FACTORY.setter + def LOGGER_FACTORY(self, value): + self._logger_factory = value @property def BACKEND_OVERRIDES(self): # Allow per-queue overrides of the backend. - return self._get('BACKEND_OVERRIDES', {}) # type: Mapping[QueueName, str] + if not self._backend_overrides: + self._backend_overrides = self._get('BACKEND_OVERRIDES', {}) + return self._backend_overrides # type: Mapping[QueueName, str] + + @BACKEND_OVERRIDES.setter + def BACKEND_OVERRIDES(self, value): + self._backend_overrides = value @property def MIDDLEWARE(self): - return self._get('MIDDLEWARE', ( - 'django_lightweight_queue.middleware.logging.LoggingMiddleware', - )) # type: Sequence[str] + if not self._middleware: + self._middleware = self._get('MIDDLEWARE', ( + 'django_lightweight_queue.middleware.logging.LoggingMiddleware', + )) + return self._middleware # type: Sequence[str] + + @MIDDLEWARE.setter + def MIDDLEWARE(self, value): + self._middleware = value @property def IGNORE_APPS(self): @@ -59,36 +94,84 @@ def IGNORE_APPS(self): # have a file called `tasks.py` within an app, but don't want # django-lightweight-queue to import that file. # Note: this _doesn't_ prevent tasks being registered from these apps. - return self._get('IGNORE_APPS', ()) # type: Sequence[str] + if not self._ignore_apps: + self._ignore_apps = self._get('IGNORE_APPS', ()) + return self._ignore_apps # type: Sequence[str] + + @IGNORE_APPS.setter + def IGNORE_APPS(self, value): + self._ignore_apps = value @property def REDIS_HOST(self): - return self._get('REDIS_HOST', '127.0.0.1') # type: str + if not self._redis_host: + self._redis_host = self._get('REDIS_HOST', '127.0.0.1') + return self._redis_host # type: str + + @REDIS_HOST.setter + def REDIS_HOST(self, value): + self._redis_host = value @property def REDIS_PORT(self): - return self._get('REDIS_PORT', 6379) # type: int + if not self._redis_port: + self._redis_port = self._get('REDIS_PORT', 6379) + return self._redis_port # type: int + + @REDIS_PORT.setter + def REDIS_PORT(self, value): + self._redis_port = value @property def REDIS_PASSWORD(self): - return self._get('REDIS_PASSWORD', None) # type: Optional[str] + if not self._redis_password: + self._redis_password = self._get('REDIS_PASSWORD', None) + return self._redis_password # type: Optional[str] + + @REDIS_PASSWORD.setter + def REDIS_PASSWORD(self, value): + self._redis_password = value @property def REDIS_PREFIX(self): - return self._get('REDIS_PREFIX', '') # type: str + if not self._redis_prefix: + self._redis_prefix = self._get('REDIS_PREFIX', '') + return self._redis_prefix # type: str + + @REDIS_PREFIX.setter + def REDIS_PREFIX(self, value): + self._redis_prefix = value @property def ENABLE_PROMETHEUS(self): - return self._get('ENABLE_PROMETHEUS', False) # type: bool + if not self._enable_prometheus: + self._enable_prometheus = self._get('ENABLE_PROMETHEUS', False) + return self._enable_prometheus # type: bool + + @ENABLE_PROMETHEUS.setter + def ENABLE_PROMETHEUS(self, value): + self._enable_prometheus = value @property def PROMETHEUS_START_PORT(self): # Workers will export metrics on this port, and ports following it - return self._get('PROMETHEUS_START_PORT', 9300) # type: int + if not self._prometheus_start_port: + self._prometheus_start_port = self._get('PROMETHEUS_START_PORT', 9300) + return self._prometheus_start_port # type: int + + @PROMETHEUS_START_PORT.setter + def PROMETHEUS_START_PORT(self, value): + self._prometheus_start_port = value @property def ATOMIC_JOBS(self): - return self._get('ATOMIC_JOBS', True) # type: bool + if not self._atomic_jobs: + self._atomic_jobs = self._get('ATOMIC_JOBS', True) + return self._atomic_jobs # type: bool + + @ATOMIC_JOBS.setter + def ATOMIC_JOBS(self, value): + self._atomic_jobs = value settings = Settings() From 8fe395116a6c156886abdf7dcf6811bd3c548f95 Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Fri, 24 Jun 2022 19:17:32 -0400 Subject: [PATCH 04/21] fix missing type hint and revert name to app_settings --- django_lightweight_queue/app_settings.py | 8 +++---- django_lightweight_queue/backends/redis.py | 12 +++++----- .../backends/reliable_redis.py | 12 +++++----- django_lightweight_queue/exposition.py | 6 ++--- .../commands/queue_configuration.py | 4 ++-- django_lightweight_queue/runner.py | 6 ++--- django_lightweight_queue/task.py | 4 ++-- django_lightweight_queue/utils.py | 22 +++++++++---------- django_lightweight_queue/worker.py | 8 +++---- 9 files changed, 41 insertions(+), 41 deletions(-) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index 3cf40a5..6b59e58 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -1,4 +1,4 @@ -from typing import Union, Mapping, TypeVar, Callable, Optional, Sequence +from typing import Union, Mapping, TypeVar, Callable, Optional, Sequence, Dict from django.conf import settings as django_settings @@ -8,7 +8,7 @@ T = TypeVar('T') -class Settings(): +class Settings: def _get(self, suffix: str, default: T) -> T: attr_name = '{}{}'.format(constants.SETTING_NAME_PREFIX, suffix) return getattr(django_settings, attr_name, default) @@ -32,7 +32,7 @@ def _get(self, suffix: str, default: T) -> T: def WORKERS(self): if not self._workers: self._workers = self._get('WORKERS', {}) - return self._workers + return self._workers # type: Dict[QueueName, int] @WORKERS.setter def WORKERS(self, value): @@ -174,4 +174,4 @@ def ATOMIC_JOBS(self, value): self._atomic_jobs = value -settings = Settings() +app_settings = Settings() diff --git a/django_lightweight_queue/backends/redis.py b/django_lightweight_queue/backends/redis.py index 72b5767..4a136f5 100644 --- a/django_lightweight_queue/backends/redis.py +++ b/django_lightweight_queue/backends/redis.py @@ -7,7 +7,7 @@ from .base import BackendWithPauseResume from ..types import QueueName, WorkerNumber from ..utils import block_for_time -from ..app_settings import settings +from ..app_settings import app_settings class RedisBackend(BackendWithPauseResume): @@ -17,9 +17,9 @@ class RedisBackend(BackendWithPauseResume): def __init__(self) -> None: self.client = redis.StrictRedis( - host=settings.REDIS_HOST, - port=settings.REDIS_PORT, - password=settings.REDIS_PASSWORD, + host=app_settings.REDIS_HOST, + port=app_settings.REDIS_PORT, + password=app_settings.REDIS_PASSWORD, ) def enqueue(self, job: Job, queue: QueueName) -> None: @@ -79,9 +79,9 @@ def is_paused(self, queue: QueueName) -> bool: return bool(self.client.exists(self._pause_key(queue))) def _key(self, queue: QueueName) -> str: - if settings.REDIS_PREFIX: + if app_settings.REDIS_PREFIX: return '{}:django_lightweight_queue:{}'.format( - settings.REDIS_PREFIX, + app_settings.REDIS_PREFIX, queue, ) diff --git a/django_lightweight_queue/backends/reliable_redis.py b/django_lightweight_queue/backends/reliable_redis.py index b01b5cc..9eca0ad 100644 --- a/django_lightweight_queue/backends/reliable_redis.py +++ b/django_lightweight_queue/backends/reliable_redis.py @@ -7,7 +7,7 @@ from .base import BackendWithDeduplicate, BackendWithPauseResume from ..types import QueueName, WorkerNumber from ..utils import block_for_time, get_worker_numbers -from ..app_settings import settings +from ..app_settings import app_settings from ..progress_logger import ProgressLogger, NULL_PROGRESS_LOGGER # Work around https://github.com/python/mypy/issues/9914. Name needs to match @@ -39,9 +39,9 @@ class ReliableRedisBackend(BackendWithDeduplicate, BackendWithPauseResume): def __init__(self) -> None: self.client = redis.StrictRedis( - host=settings.REDIS_HOST, - port=settings.REDIS_PORT, - password=settings.REDIS_PASSWORD, + host=app_settings.REDIS_HOST, + port=app_settings.REDIS_PORT, + password=app_settings.REDIS_PASSWORD, ) def startup(self, queue: QueueName) -> None: @@ -245,9 +245,9 @@ def _processing_key(self, queue: QueueName, worker_number: WorkerNumber) -> str: return self._prefix_key(key) def _prefix_key(self, key: str) -> str: - if settings.REDIS_PREFIX: + if app_settings.REDIS_PREFIX: return '{}:{}'.format( - settings.REDIS_PREFIX, + app_settings.REDIS_PREFIX, key, ) diff --git a/django_lightweight_queue/exposition.py b/django_lightweight_queue/exposition.py index 9ebba15..53fa53c 100644 --- a/django_lightweight_queue/exposition.py +++ b/django_lightweight_queue/exposition.py @@ -7,7 +7,7 @@ from prometheus_client.exposition import MetricsHandler from .types import QueueName, WorkerNumber -from .app_settings import settings +from .app_settings import app_settings def get_config_response( @@ -23,7 +23,7 @@ def get_config_response( "targets": [ "{}:{}".format( gethostname(), - settings.PROMETHEUS_START_PORT + index, + app_settings.PROMETHEUS_START_PORT + index, ), ], "labels": { @@ -60,7 +60,7 @@ def __init__(self, *args, **kwargs): super(MetricsServer, self).__init__(*args, **kwargs) def run(self): - httpd = HTTPServer(('0.0.0.0', settings.PROMETHEUS_START_PORT), RequestHandler) + httpd = HTTPServer(('0.0.0.0', app_settings.PROMETHEUS_START_PORT), RequestHandler) httpd.timeout = 2 try: diff --git a/django_lightweight_queue/management/commands/queue_configuration.py b/django_lightweight_queue/management/commands/queue_configuration.py index 7c4aaf1..787ce98 100644 --- a/django_lightweight_queue/management/commands/queue_configuration.py +++ b/django_lightweight_queue/management/commands/queue_configuration.py @@ -3,7 +3,7 @@ from django.core.management.base import BaseCommand, CommandParser from ...utils import get_backend, get_queue_counts, load_extra_config -from ...app_settings import settings +from ...app_settings import app_settings from ...cron_scheduler import get_cron_config @@ -37,7 +37,7 @@ def handle(self, **options: Any) -> None: print("") print("Middleware:") - for x in settings.MIDDLEWARE: + for x in app_settings.MIDDLEWARE: print(" * {}".format(x)) print("") diff --git a/django_lightweight_queue/runner.py b/django_lightweight_queue/runner.py index 712c66f..11855b8 100644 --- a/django_lightweight_queue/runner.py +++ b/django_lightweight_queue/runner.py @@ -7,7 +7,7 @@ from .types import Logger, QueueName, WorkerNumber from .utils import get_backend, set_process_title from .exposition import metrics_http_server -from .app_settings import settings +from .app_settings import app_settings from .machine_types import Machine from .cron_scheduler import ( CronScheduler, @@ -64,7 +64,7 @@ def handle_term(signum: int, stack: object) -> None: for x in machine.worker_names } # type: Dict[Tuple[QueueName, WorkerNumber], Tuple[Optional[subprocess.Popen[bytes]], str]] - if settings.ENABLE_PROMETHEUS: + if app_settings.ENABLE_PROMETHEUS: metrics_server = metrics_http_server(machine.worker_names) metrics_server.start() @@ -107,7 +107,7 @@ def handle_term(signum: int, stack: object) -> None: queue, str(worker_num), '--prometheus-port', - str(settings.PROMETHEUS_START_PORT + index), + str(app_settings.PROMETHEUS_START_PORT + index), ] touch_filename = touch_filename_fn(queue) diff --git a/django_lightweight_queue/task.py b/django_lightweight_queue/task.py index 2da421f..25f57ad 100644 --- a/django_lightweight_queue/task.py +++ b/django_lightweight_queue/task.py @@ -15,7 +15,7 @@ from .job import Job from .types import QueueName from .utils import get_backend, contribute_implied_queue_name -from .app_settings import settings +from .app_settings import app_settings TCallable = TypeVar('TCallable', bound=Callable[..., Any]) @@ -82,7 +82,7 @@ def slow_fn(arg): """ if atomic is None: - atomic = settings.ATOMIC_JOBS + atomic = app_settings.ATOMIC_JOBS self.queue = QueueName(queue) self.timeout = timeout diff --git a/django_lightweight_queue/utils.py b/django_lightweight_queue/utils.py index ba712d8..802f6e3 100644 --- a/django_lightweight_queue/utils.py +++ b/django_lightweight_queue/utils.py @@ -23,7 +23,7 @@ from . import constants from .types import Logger, QueueName, WorkerNumber -from .app_settings import settings +from .app_settings import app_settings if TYPE_CHECKING: from .backends.base import BaseBackend @@ -48,7 +48,7 @@ def with_prefix(names: Iterable[str]) -> Set[str]: for name in names ) - setting_names = get_setting_names(settings) + setting_names = get_setting_names(app_settings) extra_names = get_setting_names(extra_settings) unexpected_names = extra_names - with_prefix(setting_names) @@ -59,7 +59,7 @@ def with_prefix(names: Iterable[str]) -> Set[str]: override_names = extra_names - unexpected_names for name in override_names: short_name = name[len(constants.SETTING_NAME_PREFIX):] - setattr(settings, short_name, getattr(extra_settings, name)) + setattr(app_settings, short_name, getattr(extra_settings, name)) @lru_cache() @@ -73,19 +73,19 @@ def get_path(path: str) -> Any: @lru_cache() def get_backend(queue: QueueName) -> 'BaseBackend': - return get_path(settings.BACKEND_OVERRIDES.get( + return get_path(app_settings.BACKEND_OVERRIDES.get( queue, - settings.BACKEND, + app_settings.BACKEND, ))() @lru_cache() def get_logger(name: str) -> Logger: - get_logger_fn = settings.LOGGER_FACTORY + get_logger_fn = app_settings.LOGGER_FACTORY if not callable(get_logger_fn): get_logger_fn = cast( Callable[[str], Logger], - get_path(settings.LOGGER_FACTORY), + get_path(app_settings.LOGGER_FACTORY), ) return get_logger_fn(name) @@ -94,7 +94,7 @@ def get_logger(name: str) -> Logger: def get_middleware() -> List[Any]: middleware = [] - for path in settings.MIDDLEWARE: + for path in app_settings.MIDDLEWARE: try: middleware.append(get_path(path)()) except MiddlewareNotUsed: @@ -114,12 +114,12 @@ def contribute_implied_queue_name(queue: QueueName) -> None: "Queues have already been enumerated, ensure that " "'contribute_implied_queue_name' is called during setup.", ) - settings.WORKERS.setdefault(queue, 1) + app_settings.WORKERS.setdefault(queue, 1) def get_queue_counts() -> Mapping[QueueName, int]: refuse_further_implied_queues() - return settings.WORKERS + return app_settings.WORKERS def get_worker_numbers(queue: QueueName) -> Collection[WorkerNumber]: @@ -144,7 +144,7 @@ def import_all_submodules(name: str, exclude: Sequence[str] = ()) -> None: def load_all_tasks() -> None: - import_all_submodules('tasks', settings.IGNORE_APPS) + import_all_submodules('tasks', app_settings.IGNORE_APPS) def block_for_time( diff --git a/django_lightweight_queue/worker.py b/django_lightweight_queue/worker.py index 63b9ee7..565c31e 100644 --- a/django_lightweight_queue/worker.py +++ b/django_lightweight_queue/worker.py @@ -14,10 +14,10 @@ from .types import QueueName, WorkerNumber from .utils import get_logger, get_backend, set_process_title -from .app_settings import settings +from .app_settings import app_settings from .backends.base import BaseBackend -if settings.ENABLE_PROMETHEUS: +if app_settings.ENABLE_PROMETHEUS: job_duration = Summary( 'item_processed_seconds', "Item processing time", @@ -54,7 +54,7 @@ def __init__( self.name = '{}/{}'.format(queue, worker_num) def run(self) -> None: - if settings.ENABLE_PROMETHEUS and self.prometheus_port is not None: + if app_settings.ENABLE_PROMETHEUS and self.prometheus_port is not None: self.log(logging.INFO, "Exporting metrics on port {}".format(self.prometheus_port)) start_http_server(self.prometheus_port) @@ -88,7 +88,7 @@ def run(self) -> None: item_processed = self.process(backend) post_process_time = time.time() - if settings.ENABLE_PROMETHEUS: + if app_settings.ENABLE_PROMETHEUS: job_duration.labels(self.queue).observe( post_process_time - pre_process_time, ) From 4601ca318cee810763ae2dc429ef127f795b5e25 Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Fri, 24 Jun 2022 19:18:10 -0400 Subject: [PATCH 05/21] revert version number update --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index f78737d..0c3996d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "django-lightweight-queue" -version = "4.6.0" +version = "4.5.1" description = "Lightweight & modular queue and cron system for Django" authors = ["Thread Engineering "] license = "BSD-3-Clause" From fe81a32db583fe46a64f644e5e12507ea7a4d604 Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Fri, 24 Jun 2022 23:03:18 -0400 Subject: [PATCH 06/21] run isort --- django_lightweight_queue/app_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index 6b59e58..29b43df 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -1,4 +1,4 @@ -from typing import Union, Mapping, TypeVar, Callable, Optional, Sequence, Dict +from typing import Dict, Union, Mapping, TypeVar, Callable, Optional, Sequence from django.conf import settings as django_settings From caecc022624f081b3d90bd02243552836b75e902 Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Fri, 24 Jun 2022 23:10:27 -0400 Subject: [PATCH 07/21] move readme changes to own PR --- README.md | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 68ecac1..abce973 100644 --- a/README.md +++ b/README.md @@ -15,19 +15,6 @@ backends are great candidates for community contributions. ## Basic Usage -Start by adding `django_lightweight_queue` to your `INSTALLED_APPS`: - -```python -INSTALLED_APPS = [ - "django.contrib.admin", - "django.contrib.auth", - ..., - "django_lightweight_queue", -] -``` - -After that, define your task in any file you want: - ```python import time from django_lightweight_queue import task @@ -80,12 +67,12 @@ present in the specified file are inherited from the global configuration. There are four built-in backends: -| Backend | Import Location | Type | Description | -| -------------- |:----------------------------------------------------------------------| ----------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Synchronous | django_lightweight_queue.backends.synchronous.SynchronousBackend | Development | Executes the task inline, without any actual queuing. | -| Redis | django_lightweight_queue.backends.redis.RedisBackend | Production | Executes tasks at-most-once using [Redis][redis] for storage of the enqueued tasks. | -| Reliable Redis | django_lightweight_queue.backends.reliable_redis.ReliableRedisBackend | Production | Executes tasks at-least-once using [Redis][redis] for storage of the enqueued tasks (subject to Redis consistency). Does not guarantee the task _completes_. | -| Debug Web | django_lightweight_queue.backends.debug_web.DebugWebBackend | Debugging | Instead of running jobs it prints the url to a view that can be used to run a task in a transaction which will be rolled back. This is useful for debugging and optimising tasks. | +| Backend | Type | Description | +| -------------- | ----------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Synchronous | Development | Executes the task inline, without any actual queuing. | +| Redis | Production | Executes tasks at-most-once using [Redis][redis] for storage of the enqueued tasks. | +| Reliable Redis | Production | Executes tasks at-least-once using [Redis][redis] for storage of the enqueued tasks (subject to Redis consistency). Does not guarantee the task _completes_. | +| Debug Web | Debugging | Instead of running jobs it prints the url to a view that can be used to run a task in a transaction which will be rolled back. This is useful for debugging and optimising tasks. | [redis]: https://redis.io/ From 58663f7861f486b42efe87de40c956f851c7358d Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Sat, 25 Jun 2022 00:09:25 -0400 Subject: [PATCH 08/21] appease mypy with appropriate blood-based rituals --- django_lightweight_queue/app_settings.py | 72 ++++++++++++++---------- 1 file changed, 43 insertions(+), 29 deletions(-) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index 29b43df..b23b89a 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -1,4 +1,4 @@ -from typing import Dict, Union, Mapping, TypeVar, Callable, Optional, Sequence +from typing import Any, Dict, Union, TypeVar, Callable, Optional, Sequence from django.conf import settings as django_settings @@ -28,67 +28,81 @@ def _get(self, suffix: str, default: T) -> T: _prometheus_start_port = None _atomic_jobs = None + def get_empty_dict(self) -> Dict[Any, Any]: + """ + Declare dummy type to make mypy happy. + + Mypy cannot handle types changing after instantiation, which is + exactly what happens to any setting that is a dict. This helper + method works around https://github.com/python/mypy/issues/6463 + and makes mypy happy -- at no point will we actually return + Dict[Any, Any] because between the time that mypy reads it and + the time that we actually need it, it will have been populated + with the value that we actually need it to be. + """ + return {} + @property - def WORKERS(self): + def WORKERS(self) -> Dict[QueueName, int]: if not self._workers: - self._workers = self._get('WORKERS', {}) - return self._workers # type: Dict[QueueName, int] + self._workers = self._get('WORKERS', self.get_empty_dict()) + return self._workers @WORKERS.setter def WORKERS(self, value): self._workers = value @property - def BACKEND(self): + def BACKEND(self) -> str: if not self._backend: self._backend = self._get( 'BACKEND', 'django_lightweight_queue.backends.synchronous.SynchronousBackend', ) - return self._backend # type: str + return self._backend @BACKEND.setter def BACKEND(self, value): self._backend = value @property - def LOGGER_FACTORY(self): + def LOGGER_FACTORY(self) -> Union[str, Callable[[str], Logger]]: if not self._logger_factory: self._logger_factory = self._get( 'LOGGER_FACTORY', 'logging.getLogger', ) - return self._logger_factory # type: Union[str, Callable[[str], Logger]] + return self._logger_factory @LOGGER_FACTORY.setter def LOGGER_FACTORY(self, value): self._logger_factory = value @property - def BACKEND_OVERRIDES(self): + def BACKEND_OVERRIDES(self) -> Dict[QueueName, str]: # Allow per-queue overrides of the backend. if not self._backend_overrides: - self._backend_overrides = self._get('BACKEND_OVERRIDES', {}) - return self._backend_overrides # type: Mapping[QueueName, str] + self._backend_overrides = self._get('BACKEND_OVERRIDES', self.get_empty_dict()) + return self._backend_overrides @BACKEND_OVERRIDES.setter def BACKEND_OVERRIDES(self, value): self._backend_overrides = value @property - def MIDDLEWARE(self): + def MIDDLEWARE(self) -> Sequence[str]: if not self._middleware: self._middleware = self._get('MIDDLEWARE', ( 'django_lightweight_queue.middleware.logging.LoggingMiddleware', )) - return self._middleware # type: Sequence[str] + return self._middleware @MIDDLEWARE.setter def MIDDLEWARE(self, value): self._middleware = value @property - def IGNORE_APPS(self): + def IGNORE_APPS(self) -> Sequence[str]: # Apps to ignore when looking for tasks. Apps must be specified as the dotted # name used in `INSTALLED_APPS`. This is expected to be useful when you need to # have a file called `tasks.py` within an app, but don't want @@ -96,78 +110,78 @@ def IGNORE_APPS(self): # Note: this _doesn't_ prevent tasks being registered from these apps. if not self._ignore_apps: self._ignore_apps = self._get('IGNORE_APPS', ()) - return self._ignore_apps # type: Sequence[str] + return self._ignore_apps @IGNORE_APPS.setter def IGNORE_APPS(self, value): self._ignore_apps = value @property - def REDIS_HOST(self): + def REDIS_HOST(self) -> str: if not self._redis_host: self._redis_host = self._get('REDIS_HOST', '127.0.0.1') - return self._redis_host # type: str + return self._redis_host @REDIS_HOST.setter def REDIS_HOST(self, value): self._redis_host = value @property - def REDIS_PORT(self): + def REDIS_PORT(self) -> int: if not self._redis_port: self._redis_port = self._get('REDIS_PORT', 6379) - return self._redis_port # type: int + return self._redis_port @REDIS_PORT.setter def REDIS_PORT(self, value): self._redis_port = value @property - def REDIS_PASSWORD(self): + def REDIS_PASSWORD(self) -> Optional[str]: if not self._redis_password: self._redis_password = self._get('REDIS_PASSWORD', None) - return self._redis_password # type: Optional[str] + return self._redis_password @REDIS_PASSWORD.setter def REDIS_PASSWORD(self, value): self._redis_password = value @property - def REDIS_PREFIX(self): + def REDIS_PREFIX(self) -> str: if not self._redis_prefix: self._redis_prefix = self._get('REDIS_PREFIX', '') - return self._redis_prefix # type: str + return self._redis_prefix @REDIS_PREFIX.setter def REDIS_PREFIX(self, value): self._redis_prefix = value @property - def ENABLE_PROMETHEUS(self): + def ENABLE_PROMETHEUS(self) -> bool: if not self._enable_prometheus: self._enable_prometheus = self._get('ENABLE_PROMETHEUS', False) - return self._enable_prometheus # type: bool + return self._enable_prometheus @ENABLE_PROMETHEUS.setter def ENABLE_PROMETHEUS(self, value): self._enable_prometheus = value @property - def PROMETHEUS_START_PORT(self): + def PROMETHEUS_START_PORT(self) -> int: # Workers will export metrics on this port, and ports following it if not self._prometheus_start_port: self._prometheus_start_port = self._get('PROMETHEUS_START_PORT', 9300) - return self._prometheus_start_port # type: int + return self._prometheus_start_port @PROMETHEUS_START_PORT.setter def PROMETHEUS_START_PORT(self, value): self._prometheus_start_port = value @property - def ATOMIC_JOBS(self): + def ATOMIC_JOBS(self) -> bool: if not self._atomic_jobs: self._atomic_jobs = self._get('ATOMIC_JOBS', True) - return self._atomic_jobs # type: bool + return self._atomic_jobs @ATOMIC_JOBS.setter def ATOMIC_JOBS(self, value): From cd8dc9e9c633dec6c303579e045253ffb8a40eca Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Sat, 25 Jun 2022 00:21:05 -0400 Subject: [PATCH 09/21] revert mock to use .patch.dict() --- tests/test_reliable_redis_backend.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_reliable_redis_backend.py b/tests/test_reliable_redis_backend.py index 9d5ad71..6a88210 100644 --- a/tests/test_reliable_redis_backend.py +++ b/tests/test_reliable_redis_backend.py @@ -49,8 +49,8 @@ def mock_workers(self, workers: Mapping[str, int]) -> Iterator[None]: with unittest.mock.patch( 'django_lightweight_queue.utils._accepting_implied_queues', new=False, - ), unittest.mock.patch( - 'django_lightweight_queue.app_settings.Settings.WORKERS', + ), unittest.mock.patch.dict( + 'django_lightweight_queue.app_settings.app_settings.WORKERS', workers, ): yield From dcbeb0197860351ae9bb3d2e6fcad764100597e2 Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Sun, 10 Jul 2022 17:32:32 -0400 Subject: [PATCH 10/21] Refactor for layer approach --- django_lightweight_queue/app_settings.py | 224 +++++------------------ django_lightweight_queue/utils.py | 9 +- tests/test_pause_resume.py | 2 +- tests/test_task.py | 4 +- 4 files changed, 55 insertions(+), 184 deletions(-) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index b23b89a..fbbe8db 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -1,191 +1,57 @@ -from typing import Any, Dict, Union, TypeVar, Callable, Optional, Sequence +from typing import Dict, Union, Callable, Optional, Sequence from django.conf import settings as django_settings from . import constants from .types import Logger, QueueName -T = TypeVar('T') - class Settings: - def _get(self, suffix: str, default: T) -> T: - attr_name = '{}{}'.format(constants.SETTING_NAME_PREFIX, suffix) - return getattr(django_settings, attr_name, default) - - # adjustable values at runtime - _workers = None - _backend = None - _logger_factory = None - _backend_overrides = None - _middleware = None - _ignore_apps = None - _redis_host = None - _redis_port = None - _redis_password = None - _redis_prefix = None - _enable_prometheus = None - _prometheus_start_port = None - _atomic_jobs = None - - def get_empty_dict(self) -> Dict[Any, Any]: - """ - Declare dummy type to make mypy happy. - - Mypy cannot handle types changing after instantiation, which is - exactly what happens to any setting that is a dict. This helper - method works around https://github.com/python/mypy/issues/6463 - and makes mypy happy -- at no point will we actually return - Dict[Any, Any] because between the time that mypy reads it and - the time that we actually need it, it will have been populated - with the value that we actually need it to be. - """ - return {} - - @property - def WORKERS(self) -> Dict[QueueName, int]: - if not self._workers: - self._workers = self._get('WORKERS', self.get_empty_dict()) - return self._workers - - @WORKERS.setter - def WORKERS(self, value): - self._workers = value - - @property - def BACKEND(self) -> str: - if not self._backend: - self._backend = self._get( - 'BACKEND', - 'django_lightweight_queue.backends.synchronous.SynchronousBackend', + _uses_short_names: bool = True # used later in checking for values + + +class Defaults(Settings): + WORKERS: Dict[QueueName, int] = {} + BACKEND: str = 'django_lightweight_queue.backends.synchronous.SynchronousBackend' + LOGGER_FACTORY: Union[str, Callable[[str], Logger]] = 'logging.getLogger' + BACKEND_OVERRIDES: Dict[QueueName, str] = {} + MIDDLEWARE: Sequence[str] = ('django_lightweight_queue.middleware.logging.LoggingMiddleware',) + # Apps to ignore when looking for tasks. Apps must be specified as the dotted + # name used in `INSTALLED_APPS`. This is expected to be useful when you need to + # have a file called `tasks.py` within an app, but don't want + # django-lightweight-queue to import that file. + # Note: this _doesn't_ prevent tasks being registered from these apps. + IGNORE_APPS: Sequence[str] = () + REDIS_HOST: str = '127.0.0.1' + REDIS_PORT: int = 6379 + REDIS_PASSWORD: Optional[str] = None + REDIS_PREFIX: str = "" + ENABLE_PROMETHEUS: bool = False + # Workers will export metrics on this port, and ports following it + PROMETHEUS_START_PORT: int = 9300 + ATOMIC_JOBS: bool = True + + +class AppSettings: + def __init__(self, layers: list[Settings]) -> None: + self._layers = layers + + def add_layer(self, layer: Settings) -> None: # to be called by `load_extra_config` + self._layers.append(layer) + + def __getattr__(self, name): + # reverse so that later layers override earlier ones + for layer in reversed(self._layers): + # check to see if the layer is internal or external + use_short_names = getattr(layer, "_uses_short_names", False) + attr_name = ( + '{}{}'.format(constants.SETTING_NAME_PREFIX, name) + if use_short_names else name ) - return self._backend - - @BACKEND.setter - def BACKEND(self, value): - self._backend = value - - @property - def LOGGER_FACTORY(self) -> Union[str, Callable[[str], Logger]]: - if not self._logger_factory: - self._logger_factory = self._get( - 'LOGGER_FACTORY', - 'logging.getLogger', - ) - return self._logger_factory - - @LOGGER_FACTORY.setter - def LOGGER_FACTORY(self, value): - self._logger_factory = value - - @property - def BACKEND_OVERRIDES(self) -> Dict[QueueName, str]: - # Allow per-queue overrides of the backend. - if not self._backend_overrides: - self._backend_overrides = self._get('BACKEND_OVERRIDES', self.get_empty_dict()) - return self._backend_overrides - - @BACKEND_OVERRIDES.setter - def BACKEND_OVERRIDES(self, value): - self._backend_overrides = value - - @property - def MIDDLEWARE(self) -> Sequence[str]: - if not self._middleware: - self._middleware = self._get('MIDDLEWARE', ( - 'django_lightweight_queue.middleware.logging.LoggingMiddleware', - )) - return self._middleware - - @MIDDLEWARE.setter - def MIDDLEWARE(self, value): - self._middleware = value - - @property - def IGNORE_APPS(self) -> Sequence[str]: - # Apps to ignore when looking for tasks. Apps must be specified as the dotted - # name used in `INSTALLED_APPS`. This is expected to be useful when you need to - # have a file called `tasks.py` within an app, but don't want - # django-lightweight-queue to import that file. - # Note: this _doesn't_ prevent tasks being registered from these apps. - if not self._ignore_apps: - self._ignore_apps = self._get('IGNORE_APPS', ()) - return self._ignore_apps - - @IGNORE_APPS.setter - def IGNORE_APPS(self, value): - self._ignore_apps = value - - @property - def REDIS_HOST(self) -> str: - if not self._redis_host: - self._redis_host = self._get('REDIS_HOST', '127.0.0.1') - return self._redis_host - - @REDIS_HOST.setter - def REDIS_HOST(self, value): - self._redis_host = value - - @property - def REDIS_PORT(self) -> int: - if not self._redis_port: - self._redis_port = self._get('REDIS_PORT', 6379) - return self._redis_port - - @REDIS_PORT.setter - def REDIS_PORT(self, value): - self._redis_port = value - - @property - def REDIS_PASSWORD(self) -> Optional[str]: - if not self._redis_password: - self._redis_password = self._get('REDIS_PASSWORD', None) - return self._redis_password - - @REDIS_PASSWORD.setter - def REDIS_PASSWORD(self, value): - self._redis_password = value - - @property - def REDIS_PREFIX(self) -> str: - if not self._redis_prefix: - self._redis_prefix = self._get('REDIS_PREFIX', '') - return self._redis_prefix - - @REDIS_PREFIX.setter - def REDIS_PREFIX(self, value): - self._redis_prefix = value - - @property - def ENABLE_PROMETHEUS(self) -> bool: - if not self._enable_prometheus: - self._enable_prometheus = self._get('ENABLE_PROMETHEUS', False) - return self._enable_prometheus - - @ENABLE_PROMETHEUS.setter - def ENABLE_PROMETHEUS(self, value): - self._enable_prometheus = value - - @property - def PROMETHEUS_START_PORT(self) -> int: - # Workers will export metrics on this port, and ports following it - if not self._prometheus_start_port: - self._prometheus_start_port = self._get('PROMETHEUS_START_PORT', 9300) - return self._prometheus_start_port - - @PROMETHEUS_START_PORT.setter - def PROMETHEUS_START_PORT(self, value): - self._prometheus_start_port = value - - @property - def ATOMIC_JOBS(self) -> bool: - if not self._atomic_jobs: - self._atomic_jobs = self._get('ATOMIC_JOBS', True) - return self._atomic_jobs + if hasattr(layer, attr_name): + return getattr(layer, attr_name) - @ATOMIC_JOBS.setter - def ATOMIC_JOBS(self, value): - self._atomic_jobs = value + raise AttributeError(f"Sorry, '{name}' is not a valid setting.") -app_settings = Settings() +app_settings = AppSettings(layers=[Defaults(), django_settings]) diff --git a/django_lightweight_queue/utils.py b/django_lightweight_queue/utils.py index 802f6e3..de70d3d 100644 --- a/django_lightweight_queue/utils.py +++ b/django_lightweight_queue/utils.py @@ -23,7 +23,7 @@ from . import constants from .types import Logger, QueueName, WorkerNumber -from .app_settings import app_settings +from .app_settings import Settings, app_settings if TYPE_CHECKING: from .backends.base import BaseBackend @@ -57,9 +57,14 @@ def with_prefix(names: Iterable[str]) -> Set[str]: warnings.warn("Ignoring unexpected setting(s) '{}'.".format(unexpected_str)) override_names = extra_names - unexpected_names + + overrides = Overrides() + for name in override_names: short_name = name[len(constants.SETTING_NAME_PREFIX):] - setattr(app_settings, short_name, getattr(extra_settings, name)) + setattr(overrides, short_name, getattr(extra_settings, name)) + + app_settings.add_layer(overrides) @lru_cache() diff --git a/tests/test_pause_resume.py b/tests/test_pause_resume.py index 842bb48..9a9efe1 100644 --- a/tests/test_pause_resume.py +++ b/tests/test_pause_resume.py @@ -52,7 +52,7 @@ def setUp(self) -> None: # Can't use override_settings due to the copying of the settings values into # module values at startup. @mock.patch( - 'django_lightweight_queue.app_settings.Settings.BACKEND', + 'django_lightweight_queue.app_settings.Defaults.BACKEND', new='django_lightweight_queue.backends.redis.RedisBackend', ) def test_pause_resume(self) -> None: diff --git a/tests/test_task.py b/tests/test_task.py index 0fe8a29..779614b 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -30,7 +30,7 @@ def mock_workers(self, workers: Mapping[str, int]) -> Iterator[None]: 'django_lightweight_queue.utils._accepting_implied_queues', new=False, ), unittest.mock.patch.dict( - 'django_lightweight_queue.app_settings.Settings.WORKERS', + 'django_lightweight_queue.app_settings.Defaults.WORKERS', workers, ): yield @@ -54,7 +54,7 @@ def mocked_get_path(path: str) -> Any: return get_path(path) patch = mock.patch( - 'django_lightweight_queue.app_settings.Settings.BACKEND', + 'django_lightweight_queue.app_settings.Defaults.BACKEND', new='test-backend', ) patch.start() From 5ae6b6224b925d3beb8155cd4692cf8a5f1e872b Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Sun, 10 Jul 2022 17:35:19 -0400 Subject: [PATCH 11/21] add type hints for getattr --- django_lightweight_queue/app_settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index fbbe8db..7677c92 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -1,4 +1,4 @@ -from typing import Dict, Union, Callable, Optional, Sequence +from typing import Any, Dict, Union, Callable, Optional, Sequence from django.conf import settings as django_settings @@ -39,7 +39,7 @@ def __init__(self, layers: list[Settings]) -> None: def add_layer(self, layer: Settings) -> None: # to be called by `load_extra_config` self._layers.append(layer) - def __getattr__(self, name): + def __getattr__(self, name: str) -> Any: # reverse so that later layers override earlier ones for layer in reversed(self._layers): # check to see if the layer is internal or external From 0b64c18f7369f4a16b466217bc27493a9c47ca42 Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Sun, 10 Jul 2022 17:37:28 -0400 Subject: [PATCH 12/21] fix support for old python --- django_lightweight_queue/app_settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index 7677c92..4ba7aa0 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -1,4 +1,4 @@ -from typing import Any, Dict, Union, Callable, Optional, Sequence +from typing import Any, Dict, List, Union, Callable, Optional, Sequence from django.conf import settings as django_settings @@ -33,7 +33,7 @@ class Defaults(Settings): class AppSettings: - def __init__(self, layers: list[Settings]) -> None: + def __init__(self, layers: List[Settings]) -> None: self._layers = layers def add_layer(self, layer: Settings) -> None: # to be called by `load_extra_config` From 0b47d543c7d45c8f876d3e278b74c09dad9c48a2 Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Sun, 10 Jul 2022 17:39:59 -0400 Subject: [PATCH 13/21] refactor shortening correctly --- django_lightweight_queue/app_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index 4ba7aa0..97b2c1d 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -45,8 +45,8 @@ def __getattr__(self, name: str) -> Any: # check to see if the layer is internal or external use_short_names = getattr(layer, "_uses_short_names", False) attr_name = ( + name if use_short_names else '{}{}'.format(constants.SETTING_NAME_PREFIX, name) - if use_short_names else name ) if hasattr(layer, attr_name): return getattr(layer, attr_name) From be04a45c8beb8625b1855bc1186b8571daa19543 Mon Sep 17 00:00:00 2001 From: Joe Kaufeld Date: Sun, 10 Jul 2022 17:52:47 -0400 Subject: [PATCH 14/21] re-add accidentally-dropped comment --- django_lightweight_queue/app_settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index 97b2c1d..3e95c0c 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -14,6 +14,7 @@ class Defaults(Settings): WORKERS: Dict[QueueName, int] = {} BACKEND: str = 'django_lightweight_queue.backends.synchronous.SynchronousBackend' LOGGER_FACTORY: Union[str, Callable[[str], Logger]] = 'logging.getLogger' + # Allow per-queue overrides of the backend. BACKEND_OVERRIDES: Dict[QueueName, str] = {} MIDDLEWARE: Sequence[str] = ('django_lightweight_queue.middleware.logging.LoggingMiddleware',) # Apps to ignore when looking for tasks. Apps must be specified as the dotted From 30676c61ee2e13fa87ca73634953e0af15974c72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joe=E2=80=AE=20=E2=80=ACKaufeld?= Date: Wed, 13 Jul 2022 16:08:12 -0400 Subject: [PATCH 15/21] Update django_lightweight_queue/app_settings.py Co-authored-by: Peter Law --- django_lightweight_queue/app_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index 3e95c0c..bb49cac 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -37,7 +37,7 @@ class AppSettings: def __init__(self, layers: List[Settings]) -> None: self._layers = layers - def add_layer(self, layer: Settings) -> None: # to be called by `load_extra_config` + def add_layer(self, layer: Settings) -> None: self._layers.append(layer) def __getattr__(self, name: str) -> Any: From 5464e6636eb6bec19ec39aaff32c55d752b30634 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joe=E2=80=AE=20=E2=80=ACKaufeld?= Date: Wed, 13 Jul 2022 16:09:00 -0400 Subject: [PATCH 16/21] Update django_lightweight_queue/app_settings.py Co-authored-by: Peter Law --- django_lightweight_queue/app_settings.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index bb49cac..42daead 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -46,8 +46,9 @@ def __getattr__(self, name: str) -> Any: # check to see if the layer is internal or external use_short_names = getattr(layer, "_uses_short_names", False) attr_name = ( - name if use_short_names else - '{}{}'.format(constants.SETTING_NAME_PREFIX, name) + name + if use_short_names + else '{}{}'.format(constants.SETTING_NAME_PREFIX, name) ) if hasattr(layer, attr_name): return getattr(layer, attr_name) From 33554ef57f9380c84dc2297887a6031b9bb61dfe Mon Sep 17 00:00:00 2001 From: Peter Law Date: Wed, 13 Jul 2022 20:02:14 +0100 Subject: [PATCH 17/21] Reinstate typing by adding a settings protocol for local use Introduce an long-name adapter which facades away the name munging logic and allows re-use of this both for Django settings and custom settings. This simplifies AppSettings which can now focus on just layering the settings. While this doesn't gain any type checking on the settings from the layers, that was never present anyway. --- django_lightweight_queue/app_settings.py | 78 ++++++++++++++++-------- django_lightweight_queue/utils.py | 14 +---- 2 files changed, 55 insertions(+), 37 deletions(-) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index 42daead..50c995e 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -1,36 +1,69 @@ from typing import Any, Dict, List, Union, Callable, Optional, Sequence +from typing_extensions import Protocol + from django.conf import settings as django_settings from . import constants from .types import Logger, QueueName -class Settings: - _uses_short_names: bool = True # used later in checking for values +class LongNameAdapter: + def __init__(self, target: Any) -> None: + self.target = target + def __getattr__(self, name: str) -> Any: + return getattr(self.target, f'{constants.SETTING_NAME_PREFIX}{name}') + + +class Settings(Protocol): + WORKERS: Dict[QueueName, int] + BACKEND: str + LOGGER_FACTORY: Union[str, Callable[[str], Logger]] -class Defaults(Settings): - WORKERS: Dict[QueueName, int] = {} - BACKEND: str = 'django_lightweight_queue.backends.synchronous.SynchronousBackend' - LOGGER_FACTORY: Union[str, Callable[[str], Logger]] = 'logging.getLogger' # Allow per-queue overrides of the backend. - BACKEND_OVERRIDES: Dict[QueueName, str] = {} - MIDDLEWARE: Sequence[str] = ('django_lightweight_queue.middleware.logging.LoggingMiddleware',) + BACKEND_OVERRIDES: Dict[QueueName, str] + MIDDLEWARE: Sequence[str] + # Apps to ignore when looking for tasks. Apps must be specified as the dotted # name used in `INSTALLED_APPS`. This is expected to be useful when you need to # have a file called `tasks.py` within an app, but don't want # django-lightweight-queue to import that file. # Note: this _doesn't_ prevent tasks being registered from these apps. - IGNORE_APPS: Sequence[str] = () - REDIS_HOST: str = '127.0.0.1' - REDIS_PORT: int = 6379 - REDIS_PASSWORD: Optional[str] = None - REDIS_PREFIX: str = "" - ENABLE_PROMETHEUS: bool = False + IGNORE_APPS: Sequence[str] + + REDIS_HOST: str + REDIS_PORT: int + REDIS_PASSWORD: Optional[str] + REDIS_PREFIX: str + + ENABLE_PROMETHEUS: bool # Workers will export metrics on this port, and ports following it - PROMETHEUS_START_PORT: int = 9300 - ATOMIC_JOBS: bool = True + PROMETHEUS_START_PORT: int + + ATOMIC_JOBS: bool + + +class Defaults(Settings): + WORKERS: Dict[QueueName, int] = {} + BACKEND = 'django_lightweight_queue.backends.synchronous.SynchronousBackend' + LOGGER_FACTORY = 'logging.getLogger' + + BACKEND_OVERRIDES: Dict[QueueName, str] = {} + MIDDLEWARE = ('django_lightweight_queue.middleware.logging.LoggingMiddleware',) + + IGNORE_APPS: Sequence[str] = () + + REDIS_HOST = '127.0.0.1' + REDIS_PORT = 6379 + REDIS_PASSWORD = None + REDIS_PREFIX = "" + + ENABLE_PROMETHEUS = False + + PROMETHEUS_START_PORT = 9300 + + ATOMIC_JOBS = True class AppSettings: @@ -43,17 +76,10 @@ def add_layer(self, layer: Settings) -> None: def __getattr__(self, name: str) -> Any: # reverse so that later layers override earlier ones for layer in reversed(self._layers): - # check to see if the layer is internal or external - use_short_names = getattr(layer, "_uses_short_names", False) - attr_name = ( - name - if use_short_names - else '{}{}'.format(constants.SETTING_NAME_PREFIX, name) - ) - if hasattr(layer, attr_name): - return getattr(layer, attr_name) + if hasattr(layer, name): + return getattr(layer, name) raise AttributeError(f"Sorry, '{name}' is not a valid setting.") -app_settings = AppSettings(layers=[Defaults(), django_settings]) +app_settings: Settings = AppSettings(layers=[Defaults(), LongNameAdapter(django_settings)]) diff --git a/django_lightweight_queue/utils.py b/django_lightweight_queue/utils.py index de70d3d..1edb6a4 100644 --- a/django_lightweight_queue/utils.py +++ b/django_lightweight_queue/utils.py @@ -23,7 +23,7 @@ from . import constants from .types import Logger, QueueName, WorkerNumber -from .app_settings import Settings, app_settings +from .app_settings import Defaults, AppSettings, app_settings, LongNameAdapter if TYPE_CHECKING: from .backends.base import BaseBackend @@ -48,7 +48,7 @@ def with_prefix(names: Iterable[str]) -> Set[str]: for name in names ) - setting_names = get_setting_names(app_settings) + setting_names = get_setting_names(Defaults()) extra_names = get_setting_names(extra_settings) unexpected_names = extra_names - with_prefix(setting_names) @@ -56,15 +56,7 @@ def with_prefix(names: Iterable[str]) -> Set[str]: unexpected_str = "' ,'".join(unexpected_names) warnings.warn("Ignoring unexpected setting(s) '{}'.".format(unexpected_str)) - override_names = extra_names - unexpected_names - - overrides = Overrides() - - for name in override_names: - short_name = name[len(constants.SETTING_NAME_PREFIX):] - setattr(overrides, short_name, getattr(extra_settings, name)) - - app_settings.add_layer(overrides) + cast(AppSettings, app_settings).add_layer(LongNameAdapter(extra_settings)) @lru_cache() From ccd9992c5672b7af9d397109db0a8f577049ea55 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 6 Aug 2022 21:51:36 +0100 Subject: [PATCH 18/21] Update the extra-config tests for the new way that settings behave The previous approach of reloading the settings module worked ok when the module itself was what was imported & mutated, however with the move to a layered approach that's no longer the case. --- tests/test_extra_config.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/test_extra_config.py b/tests/test_extra_config.py index 1f11a97..fc739d4 100644 --- a/tests/test_extra_config.py +++ b/tests/test_extra_config.py @@ -1,6 +1,6 @@ -import importlib from typing import Optional from pathlib import Path +from unittest import mock from django.test import SimpleTestCase @@ -27,11 +27,19 @@ def length(self, queue: QueueName) -> int: class ExtraConfigTests(SimpleTestCase): def setUp(self) -> None: get_backend.cache_clear() + + self.settings: app_settings.Settings = app_settings.AppSettings([app_settings.Defaults()]) + self._settings_patch = mock.patch( + 'django_lightweight_queue.utils.app_settings', + new=self.settings, + ) + self._settings_patch.start() + super().setUp() def tearDown(self) -> None: - importlib.reload(app_settings) get_backend.cache_clear() + self._settings_patch.stop() super().tearDown() def test_updates_configuration(self) -> None: @@ -40,20 +48,20 @@ def test_updates_configuration(self) -> None: backend = get_backend('test-queue') self.assertIsInstance(backend, TestBackend) - self.assertEqual('a very bad password', app_settings.REDIS_PASSWORD) + self.assertEqual('a very bad password', self.settings.REDIS_PASSWORD) def test_warns_about_unexpected_settings(self) -> None: with self.assertWarnsRegex(Warning, r'Ignoring unexpected setting.+\bNOT_REDIS_PASSWORD\b'): load_extra_config(str(TESTS_DIR / '_demo_extra_config_unexpected.py')) - self.assertEqual('expected', app_settings.REDIS_PASSWORD) + self.assertEqual('expected', self.settings.REDIS_PASSWORD) def test_updates_configuration_with_falsey_values(self) -> None: load_extra_config(str(TESTS_DIR / '_demo_extra_config.py')) load_extra_config(str(TESTS_DIR / '_demo_extra_config_falsey.py')) - self.assertIsNone(app_settings.REDIS_PASSWORD) - self.assertFalse(app_settings.ATOMIC_JOBS) + self.assertIsNone(self.settings.REDIS_PASSWORD) + self.assertFalse(self.settings.ATOMIC_JOBS) def test_rejects_missing_file(self) -> None: with self.assertRaises(FileNotFoundError): From 31eb204197ded8d3c9afb5446e4eb7ab0a5d8898 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 6 Aug 2022 21:57:21 +0100 Subject: [PATCH 19/21] Use override_settings in tests where we can now do so This slightly changes the behaviour of the mock_workers helpers, however the default is an empty dictionary so this feels like a reasonable trade-off for using the more standard override_settings util. --- tests/test_pause_resume.py | 11 ++++------- tests/test_reliable_redis_backend.py | 9 +++++---- tests/test_task.py | 16 ++++++---------- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/tests/test_pause_resume.py b/tests/test_pause_resume.py index 9a9efe1..b33c265 100644 --- a/tests/test_pause_resume.py +++ b/tests/test_pause_resume.py @@ -1,11 +1,11 @@ import io import datetime -import unittest from unittest import mock import fakeredis import freezegun +from django.test import SimpleTestCase, override_settings from django.core.management import call_command, CommandError from django_lightweight_queue.types import QueueName @@ -18,7 +18,7 @@ ) -class PauseResumeTests(unittest.TestCase): +class PauseResumeTests(SimpleTestCase): longMessage = True def assertPaused(self, queue: QueueName, context: str) -> None: @@ -49,11 +49,8 @@ def setUp(self) -> None: super().setUp() - # Can't use override_settings due to the copying of the settings values into - # module values at startup. - @mock.patch( - 'django_lightweight_queue.app_settings.Defaults.BACKEND', - new='django_lightweight_queue.backends.redis.RedisBackend', + @override_settings( + LIGHTWEIGHT_QUEUE_BACKEND='django_lightweight_queue.backends.redis.RedisBackend', ) def test_pause_resume(self) -> None: QUEUE = QueueName('test-pauseable-queue') diff --git a/tests/test_reliable_redis_backend.py b/tests/test_reliable_redis_backend.py index 6a88210..b65e87f 100644 --- a/tests/test_reliable_redis_backend.py +++ b/tests/test_reliable_redis_backend.py @@ -8,6 +8,8 @@ import fakeredis +from django.test import SimpleTestCase, override_settings + from django_lightweight_queue.job import Job from django_lightweight_queue.types import QueueName from django_lightweight_queue.backends.reliable_redis import ( @@ -18,7 +20,7 @@ from .mixins import RedisCleanupMixin -class ReliableRedisDeduplicationTests(RedisCleanupMixin, unittest.TestCase): +class ReliableRedisDeduplicationTests(RedisCleanupMixin, SimpleTestCase): longMessage = True prefix = settings.LIGHTWEIGHT_QUEUE_REDIS_PREFIX @@ -49,9 +51,8 @@ def mock_workers(self, workers: Mapping[str, int]) -> Iterator[None]: with unittest.mock.patch( 'django_lightweight_queue.utils._accepting_implied_queues', new=False, - ), unittest.mock.patch.dict( - 'django_lightweight_queue.app_settings.app_settings.WORKERS', - workers, + ), override_settings( + LIGHTWEIGHT_QUEUE_WORKERS=workers, ): yield diff --git a/tests/test_task.py b/tests/test_task.py index 779614b..742f949 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -5,6 +5,8 @@ import fakeredis +from django.test import SimpleTestCase, override_settings + from django_lightweight_queue import task from django_lightweight_queue.types import QueueName, WorkerNumber from django_lightweight_queue.utils import get_path, get_backend @@ -20,7 +22,8 @@ def dummy_task(num: int) -> None: pass -class TaskTests(unittest.TestCase): +@override_settings(LIGHTWEIGHT_QUEUE_BACKEND='test-backend') +class TaskTests(SimpleTestCase): longMessage = True prefix = settings.LIGHTWEIGHT_QUEUE_REDIS_PREFIX @@ -29,9 +32,8 @@ def mock_workers(self, workers: Mapping[str, int]) -> Iterator[None]: with unittest.mock.patch( 'django_lightweight_queue.utils._accepting_implied_queues', new=False, - ), unittest.mock.patch.dict( - 'django_lightweight_queue.app_settings.Defaults.WORKERS', - workers, + ), override_settings( + LIGHTWEIGHT_QUEUE_WORKERS=workers, ): yield @@ -53,12 +55,6 @@ def mocked_get_path(path: str) -> Any: return lambda: self.backend return get_path(path) - patch = mock.patch( - 'django_lightweight_queue.app_settings.Defaults.BACKEND', - new='test-backend', - ) - patch.start() - self.addCleanup(patch.stop) patch = mock.patch( 'django_lightweight_queue.utils.get_path', side_effect=mocked_get_path, From f86a29f03660bd1eba97dd0e3903b00693628df8 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 6 Aug 2022 22:15:28 +0100 Subject: [PATCH 20/21] Wrap longish line for clarity --- django_lightweight_queue/app_settings.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index 50c995e..3675d5e 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -82,4 +82,7 @@ def __getattr__(self, name: str) -> Any: raise AttributeError(f"Sorry, '{name}' is not a valid setting.") -app_settings: Settings = AppSettings(layers=[Defaults(), LongNameAdapter(django_settings)]) +app_settings: Settings = AppSettings(layers=[ + Defaults(), + LongNameAdapter(django_settings), +]) From 80d2fd4ab53e1dd60702d8a76fd81bb8c41d8141 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Tue, 12 Dec 2023 11:17:31 +0000 Subject: [PATCH 21/21] Clarify API boundaries and remove cast This avoids needing to cast the settings by instead documenting the expected bounds of the API offered. --- django_lightweight_queue/app_settings.py | 15 ++++++++++++++- django_lightweight_queue/utils.py | 4 ++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/django_lightweight_queue/app_settings.py b/django_lightweight_queue/app_settings.py index 8ddd4dd..002cee4 100644 --- a/django_lightweight_queue/app_settings.py +++ b/django_lightweight_queue/app_settings.py @@ -1,3 +1,11 @@ +""" +Internal API facade over Django settings. + +This provides a typed interface over Django settings as well as handling default +values for settings not set by users and supporting the addition of overrides in +some management commands. +""" + from typing import Any, Dict, List, Union, Callable, Optional, Sequence from typing_extensions import Protocol @@ -45,6 +53,11 @@ class Settings(Protocol): ATOMIC_JOBS: bool +class LayeredSettings(Settings, Protocol): + def add_layer(self, layer: Settings) -> None: + ... + + class Defaults(Settings): WORKERS: Dict[QueueName, int] = {} BACKEND = 'django_lightweight_queue.backends.synchronous.SynchronousBackend' @@ -84,7 +97,7 @@ def __getattr__(self, name: str) -> Any: raise AttributeError(f"Sorry, '{name}' is not a valid setting.") -app_settings: Settings = AppSettings(layers=[ +app_settings: LayeredSettings = AppSettings(layers=[ Defaults(), LongNameAdapter(django_settings), ]) diff --git a/django_lightweight_queue/utils.py b/django_lightweight_queue/utils.py index c9731be..e3d45b7 100644 --- a/django_lightweight_queue/utils.py +++ b/django_lightweight_queue/utils.py @@ -23,7 +23,7 @@ from . import constants from .types import Logger, QueueName, WorkerNumber -from .app_settings import Defaults, AppSettings, app_settings, LongNameAdapter +from .app_settings import Defaults, app_settings, LongNameAdapter if TYPE_CHECKING: from .backends.base import BaseBackend @@ -56,7 +56,7 @@ def with_prefix(names: Iterable[str]) -> Set[str]: unexpected_str = "' ,'".join(unexpected_names) warnings.warn("Ignoring unexpected setting(s) '{}'.".format(unexpected_str)) - cast(AppSettings, app_settings).add_layer(LongNameAdapter(extra_settings)) + app_settings.add_layer(LongNameAdapter(extra_settings)) @lru_cache()