Merge branch 'master' into allow-runtime-settings-overrides

thread · Oct 17, 2022 · f1e12d6 · f1e12d6
2 parents f86a29f + 23afd7d
commit f1e12d6
Show file tree

Hide file tree

Showing 21 changed files with 287 additions and 94 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -5,7 +5,8 @@ references:
     run:
       name: Install Poetry
       command: |
-        curl -sSL https://raw.githubusercontent.com/python-poetry/poetry/master/get-poetry.py | python
+        # Need to use version < 1.2.0 in order to support Python 3.6
+        curl -sSL https://install.python-poetry.org | python3 - --version 1.1.15
   restore-dependencies-cache: &restore-dependencies-cache
     restore_cache:
       keys:
@@ -14,7 +15,6 @@ references:
     run:
       name: Install Dependencies
       command: |
-        source $HOME/.poetry/env
         poetry install
         poetry run pip install "django~=<< parameters.django-version >>.0"
   save-dependencies-cache: &save-dependencies-cache
@@ -65,7 +65,6 @@ jobs:
       - run:
           name: Run Tests
           command: |
-            source $HOME/.poetry/env
             poetry run ./runtests
 
   lint:
@@ -82,7 +81,6 @@ jobs:
       - run:
           name: Run Flake8
           command: |
-            source $HOME/.poetry/env
             poetry run flake8
 
   type-check:
@@ -99,7 +97,6 @@ jobs:
       - run:
           name: Run Mypy
           command: |
-            source $HOME/.poetry/env
             poetry run ./script/type-check
 
   deploy:
@@ -108,11 +105,10 @@ jobs:
       version: "3.7"
     steps:
       - checkout
+      - *install-poetry
       - run:
           name: Push to PyPI
           command: |
-            curl -sSL https://raw.githubusercontent.com/python-poetry/poetry/master/get-poetry.py | python
-            source $HOME/.poetry/env
             poetry publish \
               --build \
               --username "${PYPI_USERNAME}" \

diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md
@@ -0,0 +1,21 @@
+# Development
+
+Dependencies are managed with [poetry](https://python-poetry.org/): `poetry install`
+
+Tests are run with `./runtests`
+
+## Releasing
+
+CI handles releasing to PyPI.
+Releases on GitHub are created manually.
+
+Here's how to do a release:
+
+ - Get all the desired changes into `master`
+ - Wait for CI to pass that
+ - Add a bump commit (see previous "Declare vX.Y.Z" commits; `poetry version` may be useful here)
+ - Push that commit on master
+ - Create a tag of that version number (`git tag v$(poetry version --short)`)
+ - Push the tag (`git push --tags`)
+ - CI will build & deploy that release
+ - Create a Release on GitHub, ideally with a summary of changes
diff --git 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
@@ -56,7 +69,7 @@ LIGHTWEIGHT_QUEUE_REDIS_PORT = 12345
 and then running:
 
 ```
-$ python manage.py queue_runner --config=special.py
+$ python manage.py queue_runner --extra-settings=special.py
 ```
 
 will result in the runner to use the settings from the specified configuration
@@ -67,12 +80,57 @@ 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. |
+### Synchronous (Development backend)
+
+`django_lightweight_queue.backends.synchronous.SynchronousBackend`
+
+Executes the task inline, without any actual queuing.
+
+### Redis (Production backend)
+
+`django_lightweight_queue.backends.redis.RedisBackend`
+
+Executes tasks at-most-once using [Redis][redis] for storage of the enqueued tasks.
+
+### Reliable Redis (Production backend)
+
+`django_lightweight_queue.backends.reliable_redis.ReliableRedisBackend`
+
+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 (Debug backend)
+
+`django_lightweight_queue.backends.debug_web.DebugWebBackend`
+
+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.
+
+Use this to append the appropriate URLs to the bottom of your root `urls.py`:
+
+```python
+from django.conf import settings
+from django.urls import path, include
+
+urlpatterns = [
+    ...
+]
+
+if settings.DEBUG:
+    urlpatterns += [
+        path(
+            "",
+            include(
+                "django_lightweight_queue.urls", namespace="django-lightweight-queue"
+            ),
+        )
+    ]
+```
+
+This backend may require an extra setting if your debug site is not on localhost:
+
+```python
+# defaults to http://localhost:8000
+LIGHTWEIGHT_QUEUE_SITE_URL = "http://example.com:8000"
+```
 
 [redis]: https://redis.io/
 
@@ -91,10 +149,13 @@ part of a pool:
 $ python manage.py queue_runner --machine 2 --of 4
 ```
 
-Alternatively a runner can be told explicitly which configuration to use:
+Alternatively a runner can be told explicitly how to behave by having
+extra settings loaded (any `LIGHTWEIGHT_QUEUE_*` constants found in the file
+will replace equivalent django settings) and being configured to run exactly as
+the settings describe:
 
 ```
-$ python manage.py queue_runner --exact-configuration --config=special.py
+$ python manage.py queue_runner --exact-configuration --extra-settings=special.py
 ```
 
 When using `--exact-configuration` the number of workers is configured exactly,
@@ -130,7 +191,7 @@ $ python manage.py queue_runner --machine 3 --of 3
 will result in one worker for `queue1` on the current machine, while:
 
 ```
-$ python manage.py queue_runner --exact-configuration --config=special.py
+$ python manage.py queue_runner --exact-configuration --extra-settings=special.py
 ```
 
 will result in two workers on the current machine.

diff --git a/django_lightweight_queue/cron_scheduler.py b/django_lightweight_queue/cron_scheduler.py
@@ -144,7 +144,7 @@ def get_matcher(minval, maxval, t):
                 # No module, move on.
                 continue
 
-        app_cron_config: List[CronConfig] = mod.CONFIG  # type: ignore[attr-defined]
+        app_cron_config: List[CronConfig] = mod.CONFIG
         for row in app_cron_config:
             row['min_matcher'] = get_matcher(0, 59, row.get('minutes'))
             row['hour_matcher'] = get_matcher(0, 23, row.get('hours'))

diff --git a/django_lightweight_queue/management/commands/queue_configuration.py b/django_lightweight_queue/management/commands/queue_configuration.py
@@ -1,26 +1,46 @@
+import warnings
 from typing import Any
 
 from django.core.management.base import BaseCommand, CommandParser
 
-from ...utils import get_backend, get_queue_counts, load_extra_config
+from ...utils import get_backend, get_queue_counts, load_extra_settings
+from ...constants import SETTING_NAME_PREFIX
 from ...app_settings import app_settings
 from ...cron_scheduler import get_cron_config
 
 
 class Command(BaseCommand):
     def add_arguments(self, parser: CommandParser) -> None:
-        parser.add_argument(
+        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",
+            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(self, **options: Any) -> None:
-        # Configuration overrides
-        extra_config = options['config']
+        extra_config = options.pop('config')
         if extra_config is not None:
-            load_extra_config(extra_config)
+            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)
 
         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
@@ -1,3 +1,4 @@
+import warnings
 from typing import Any, Dict, Optional
 
 import daemonize
@@ -10,8 +11,14 @@
 )
 
 from ...types import QueueName
-from ...utils import get_logger, get_backend, get_middleware, load_extra_config
+from ...utils import (
+    get_logger,
+    get_backend,
+    get_middleware,
+    load_extra_settings,
+)
 from ...runner import runner
+from ...constants import SETTING_NAME_PREFIX
 from ...machine_types import Machine, PooledMachine, DirectlyConfiguredMachine
 
 
@@ -51,23 +58,42 @@ def add_arguments(self, parser: CommandParser) -> None:
             default=None,
             help="Only run the given queue, useful for local debugging",
         )
-        parser.add_argument(
+        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",
+            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',
             help="Run queues on this machine exactly as specified. Requires the"
-                 " use of the '--config' option in addition. It is an error to"
-                 " use this option together with either '--machine' or '--of'.",
+                 " use of the '--extra-settings' option in addition. It is an"
+                 " error to use this option together with either '--machine' or"
+                 " '--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
+
         if options['exact_configuration']:
-            if not options['config']:
+            if not options['extra_settings']:
                 raise CommandError(
                     "Must provide a value for '--config' when using "
                     "'--exact-configuration'.",
@@ -110,9 +136,9 @@ def touch_filename(name: str) -> Optional[str]:
                 return None
 
         # Configuration overrides
-        extra_config = options['config']
+        extra_config = options['extra_settings']
         if extra_config is not None:
-            load_extra_config(extra_config)
+            load_extra_settings(extra_config)
 
         logger.info("Starting queue master")
 

diff --git a/django_lightweight_queue/runner.py b/django_lightweight_queue/runner.py
@@ -43,7 +43,7 @@ def runner(
 
     # Note: we deliberately configure our handling of SIGTERM _after_ the
     # startup processes have happened; this ensures that the startup processes
-    # (which could take a long time) are naturally interupted by the signal.
+    # (which could take a long time) are naturally interrupted by the signal.
     def handle_term(signum: int, stack: object) -> None:
         nonlocal running
         logger.debug("Caught TERM signal")

diff --git a/django_lightweight_queue/urls.py b/django_lightweight_queue/urls.py
@@ -1,9 +1,9 @@
-from django.conf.urls import url
+from django.urls import path
 
 from . import views
 
 app_name = 'django_lightweight_queue'
 
 urlpatterns = (
-    url(r'^debug/django-lightweight-queue/debug-run$', views.debug_run, name='debug-run'),
+    path(r'debug/django-lightweight-queue/debug-run', views.debug_run, name='debug-run'),
 )
diff --git a/django_lightweight_queue/utils.py b/django_lightweight_queue/utils.py
@@ -33,7 +33,7 @@
 FIVE_SECONDS = datetime.timedelta(seconds=5)
 
 
-def load_extra_config(file_path: str) -> None:
+def load_extra_settings(file_path: str) -> None:
     # Based on https://docs.python.org/3/library/importlib.html#importing-a-source-file-directly
     spec = importlib.util.spec_from_file_location('extra_settings', file_path)
     extra_settings = importlib.util.module_from_spec(spec)  # type: ignore[arg-type]

diff --git a/django_lightweight_queue/worker.py b/django_lightweight_queue/worker.py
@@ -159,13 +159,25 @@ def configure_cancellation(self, timeout: Optional[int], sigkill_on_stop: bool)
             signal.signal(signal.SIGUSR2, self._handle_sigusr2)
 
         if timeout is not None:
+            signal.signal(signal.SIGALRM, self._handle_alarm)
             # alarm(3) takes whole seconds
             alarm_duration = int(math.ceil(timeout))
             signal.alarm(alarm_duration)
         else:
             # Cancel any scheduled alarms
             signal.alarm(0)
 
+    def _handle_alarm(self, signal_number: int, frame: object) -> None:
+        # Log for observability
+        self.log(logging.ERROR, "Alarm received: job has timed out")
+
+        # Disconnect ourselves then re-signal so that Python does what it
+        # normally would. We could raise an exception here, however raising
+        # exceptions from signal handlers is generally discouraged.
+        signal.signal(signal.SIGALRM, signal.SIG_DFL)
+        # TODO(python-upgrade): use signal.raise_signal on Python 3.8+
+        os.kill(os.getpid(), signal.SIGALRM)
+
     def set_process_title(self, *titles: str) -> None:
         set_process_title(self.name, *titles)