feat(operations): worker concurrency and job retention settings

appsettings/migrations/0018_worker_settings.py

@@ -0,0 +1,33 @@
+from django.core.management.color import no_style
+from django.db import migrations
+
+
+SETTINGS = [
+    ("Worker Concurrency", "WORKER_CONCURRENCY", "4", "Jobs the worker runs at the same time (1-16)"),
+    ("Job Retention Days", "JOB_RETENTION_DAYS", "30", "Days finished jobs stay in the job history (1-3650)"),
+]
+
+
+def add_settings(apps, schema_editor):
+    setting = apps.get_model("appsettings", "AppSettings")
+    db_alias = schema_editor.connection.alias
+    # see 0014: move a PostgreSQL id sequence past the fixed ids first
+    with schema_editor.connection.cursor() as cursor:
+        for sql in schema_editor.connection.ops.sequence_reset_sql(no_style(), [setting]):
+            cursor.execute(sql)
+    for name, key, value, description in SETTINGS:
+        setting.objects.using(db_alias).get_or_create(
+            key=key, defaults={"name": name, "value": value, "choices": "", "description": description}
+        )
+
+
+def del_settings(apps, schema_editor):
+    setting = apps.get_model("appsettings", "AppSettings")
+    db_alias = schema_editor.connection.alias
+    setting.objects.using(db_alias).filter(key__in=[key for _, key, _, _ in SETTINGS]).delete()
+
+
+class Migration(migrations.Migration):
+    dependencies = [("appsettings", "0017_snapshot_free_space_settings")]
+
+    operations = [migrations.RunPython(add_settings, del_settings)]

appsettings/tests/test_appsettings.py

@@ -1,3 +1,4 @@
+from django.contrib.messages import get_messages
 from django.test import RequestFactory, TestCase
 from django.urls import reverse
 
@@ -48,3 +49,8 @@ class AppSettingsTestCase(TestCase):
         # a clone stays on the host of the VM it was made from; the setting
         # that sent it to a random host is removed by migration 0015
         self.assertFalse(AppSettings.objects.filter(key="CLONE_INSTANCE_AUTO_MIGRATE").exists())
+
+    def test_worker_settings_refuse_values_out_of_bounds(self):
+        response = self.client.post(reverse("appsettings"), {"WORKER_CONCURRENCY": "0"})
+        self.assertEqual(AppSettings.objects.get(key="WORKER_CONCURRENCY").value, "4")
+        self.assertTrue(any("between 1 and 16" in str(m) for m in get_messages(response.wsgi_request)))

appsettings/tests/test_migrations.py

@@ -51,3 +51,13 @@ class SnapshotFreeSpaceSettingsTestCase(TestCase):
                 limit = AppSettings._meta.get_field(field).max_length
                 if limit:
                     self.assertLessEqual(len(getattr(setting, field)), limit, f"{key} {field}")
+
+
+class WorkerSettingsTestCase(TestCase):
+    def test_the_rows_fit_their_columns(self):
+        for key in ("WORKER_CONCURRENCY", "JOB_RETENTION_DAYS"):
+            setting = AppSettings.objects.get(key=key)
+            for field in ("name", "key", "value", "description"):
+                limit = AppSettings._meta.get_field(field).max_length
+                if limit:
+                    self.assertLessEqual(len(getattr(setting, field)), limit, f"{key} {field}")

appsettings/views.py

@@ -9,6 +9,7 @@ from django.http import HttpResponseRedirect
 from django.shortcuts import render
 from django.utils.translation import gettext_noop as _
 from logs.views import addlogmsg
+from operations.utils import BOUNDS
 
 from appsettings.models import AppSettings
 
@@ -112,6 +113,13 @@ def appsettings(request):
 
         for setting in appsettings:
             if setting.key in request.POST:
+                if setting.key in BOUNDS:
+                    low, high, _default = BOUNDS[setting.key]
+                    raw = request.POST.get(setting.key, "")
+                    if not (raw.isascii() and raw.isdigit() and len(raw) <= 4 and low <= int(raw) <= high):
+                        messages.error(request, _("%(setting)s must be a whole number between %(low)s and %(high)s")
+                                       % {"setting": setting.name, "low": low, "high": high})
+                        return HttpResponseRedirect(request.get_full_path())
                 try:
                     setting.value = request.POST.get(setting.key, "")
                     setting.save()

operations/tests/test_conf.py

@@ -0,0 +1,13 @@
+from django.test import TestCase
+
+from appsettings.models import AppSettings
+from operations import utils
+
+
+class ConfTestCase(TestCase):
+    def test_defaults_and_bounds(self):
+        self.assertEqual((utils.concurrency(), utils.retention_days()), (4, 30))
+        AppSettings.objects.filter(key="WORKER_CONCURRENCY").update(value="99")
+        self.assertEqual(utils.concurrency(), 4)
+        AppSettings.objects.filter(key="WORKER_CONCURRENCY").update(value="8")
+        self.assertEqual(utils.concurrency(), 8)

operations/utils.py

@@ -1,6 +1,8 @@
 """Secrets never reach a job record, a response or a log line. What a refused
 request says about the job holding the resource: details for its owner and
-superusers, a minimal summary for everyone else."""
+superusers, a minimal summary for everyone else. Worker settings from the
+Settings page. The worker reads them itself (the settings middleware only runs
+in gunicorn)."""
 
 import re
 
@@ -8,6 +10,8 @@ from django.utils import timezone
 from django.utils.timesince import timesince
 from django.utils.translation import gettext as _
 
+from appsettings.models import AppSettings
+
 from . import jobs
 
 _NAMES = r"(?:pass|secret|token|key)\w*"
@@ -21,6 +25,8 @@ _PATTERNS = [
 ]
 _SECRET_KEY = re.compile(r"pass|secret|token|private|key$", re.I)
 
+BOUNDS = {"WORKER_CONCURRENCY": (1, 16, 4), "JOB_RETENTION_DAYS": (1, 3650, 30)}
+
 
 def safe_text(text):
     text = str(text)
@@ -60,3 +66,20 @@ def busy_message(busy, user):
         return _("%(label)s is %(state)s on this resource (started by %(user)s, %(since)s ago); "
                  "see the Jobs page.") % {**values, "user": op.username}
     return _("%(label)s is %(state)s on this resource (since %(since)s).") % values
+
+
+def _value(key):
+    low, high, default = BOUNDS[key]
+    try:
+        value = int(AppSettings.objects.get(key=key).value)
+    except (AppSettings.DoesNotExist, ValueError):
+        return default
+    return value if low <= value <= high else default
+
+
+def concurrency():
+    return _value("WORKER_CONCURRENCY")
+
+
+def retention_days():
+    return _value("JOB_RETENTION_DAYS")