Define priority range and semantics (#70)

Author: RealOrangeOne

README.md

@@ -57,7 +57,7 @@ def calculate_meaning_of_life() -> int:
 
 The task decorator accepts a few arguments to customize the task:
 
-- `priority`: The priority of the task (larger numbers are higher priority)
+- `priority`: The priority of the task (between -100 and 100. Larger numbers are higher priority. 0 by default)
 - `queue_name`: Whether to run the task on a specific queue
 - `backend`: Name of the backend for this task to use (as defined in `TASKS`)
 

django_tasks/backends/base.py

@@ -8,7 +8,7 @@
 from typing_extensions import ParamSpec
 
 from django_tasks.exceptions import InvalidTaskError
-from django_tasks.task import Task, TaskResult
+from django_tasks.task import MAX_PRIORITY, MIN_PRIORITY, Task, TaskResult
 from django_tasks.utils import is_global_function
 
 T = TypeVar("T")
@@ -45,8 +45,14 @@ def validate_task(self, task: Task) -> None:
         if not self.supports_async_task and iscoroutinefunction(task.func):
             raise InvalidTaskError("Backend does not support async tasks")
 
-        if task.priority < 0:
-            raise InvalidTaskError("priority must be zero or greater")
+        if (
+            task.priority < MIN_PRIORITY
+            or task.priority > MAX_PRIORITY
+            or int(task.priority) != task.priority
+        ):
+            raise InvalidTaskError(
+                f"priority must be a whole number between {MIN_PRIORITY} and {MAX_PRIORITY}"
+            )
 
         if not self.supports_defer and task.run_after is not None:
             raise InvalidTaskError("Backend does not support run_after")

django_tasks/backends/database/migrations/0005_alter_dbtaskresult_priority_and_more.py

@@ -0,0 +1,23 @@
+# Generated by Django 4.2.13 on 2024-07-10 15:48
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("django_tasks_database", "0004_dbtaskresult_started_at"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="dbtaskresult",
+            name="priority",
+            field=models.IntegerField(default=0),
+        ),
+        migrations.AddConstraint(
+            model_name="dbtaskresult",
+            constraint=models.CheckConstraint(
+                check=models.Q(("priority__range", (-100, 100))), name="priority_range"
+            ),
+        ),
+    ]

django_tasks/backends/database/models.py

@@ -4,12 +4,20 @@
 
 from django.core.exceptions import SuspiciousOperation
 from django.db import models
-from django.db.models import F
+from django.db.models import F, Q
+from django.db.models.constraints import CheckConstraint
 from django.utils import timezone
 from django.utils.module_loading import import_string
 from typing_extensions import ParamSpec
 
-from django_tasks.task import DEFAULT_QUEUE_NAME, ResultStatus, Task
+from django_tasks.task import (
+    DEFAULT_PRIORITY,
+    DEFAULT_QUEUE_NAME,
+    MAX_PRIORITY,
+    MIN_PRIORITY,
+    ResultStatus,
+    Task,
+)
 from django_tasks.utils import exception_to_dict, retry
 
 logger = logging.getLogger("django_tasks.backends.database")
@@ -72,7 +80,7 @@ class DBTaskResult(GenericBase[P, T], models.Model):
 
     args_kwargs = models.JSONField()
 
-    priority = models.PositiveSmallIntegerField(default=0)
+    priority = models.IntegerField(default=DEFAULT_PRIORITY)
 
     task_path = models.TextField()
 
@@ -89,6 +97,12 @@ class Meta:
         ordering = [F("priority").desc(), F("run_after").desc(nulls_last=True)]
         verbose_name = "Task Result"
         verbose_name_plural = "Task Results"
+        constraints = [
+            CheckConstraint(
+                check=Q(priority__range=(MIN_PRIORITY, MAX_PRIORITY)),
+                name="priority_range",
+            )
+        ]
 
     @property
     def task(self) -> Task[P, T]:

django_tasks/task.py

@@ -28,6 +28,9 @@
 
 DEFAULT_TASK_BACKEND_ALIAS = "default"
 DEFAULT_QUEUE_NAME = "default"
+MIN_PRIORITY = -100
+MAX_PRIORITY = 100
+DEFAULT_PRIORITY = 0
 
 
 class ResultStatus(TextChoices):
@@ -70,6 +73,7 @@ def name(self) -> str:
 
     def using(
         self,
+        *,
         priority: Optional[int] = None,
         queue_name: Optional[str] = None,
         run_after: Optional[Union[datetime, timedelta]] = None,
@@ -163,7 +167,7 @@ def task(function: Callable[P, T], /) -> Task[P, T]: ...
 @overload
 def task(
     *,
-    priority: int = 0,
+    priority: int = DEFAULT_PRIORITY,
     queue_name: str = DEFAULT_QUEUE_NAME,
     backend: str = DEFAULT_TASK_BACKEND_ALIAS,
 ) -> Callable[[Callable[P, T]], Task[P, T]]: ...
@@ -173,7 +177,7 @@ def task(
 def task(
     function: Optional[Callable[P, T]] = None,
     *,
-    priority: int = 0,
+    priority: int = DEFAULT_PRIORITY,
     queue_name: str = DEFAULT_QUEUE_NAME,
     backend: str = DEFAULT_TASK_BACKEND_ALIAS,
 ) -> Union[Task[P, T], Callable[[Callable[P, T]], Task[P, T]]]:

tests/tests/test_database_backend.py

@@ -7,7 +7,8 @@
 
 from django.core.exceptions import SuspiciousOperation
 from django.core.management import call_command, execute_from_command_line
-from django.test import TestCase, TransactionTestCase, override_settings
+from django.db.utils import IntegrityError
+from django.test import TransactionTestCase, override_settings
 from django.urls import reverse
 from django.utils import timezone
 
@@ -29,7 +30,7 @@
         }
     }
 )
-class DatabaseBackendTestCase(TestCase):
+class DatabaseBackendTestCase(TransactionTestCase):
     def test_using_correct_backend(self) -> None:
         self.assertEqual(default_task_backend, tasks["default"])
         self.assertIsInstance(tasks["default"], DatabaseBackend)
@@ -204,6 +205,34 @@ def test_database_backend_app_missing(self) -> None:
         self.assertEqual(len(errors), 1)
         self.assertIn("django_tasks.backends.database", errors[0].hint)
 
+    def test_priority_range_check(self) -> None:
+        with self.assertRaises(IntegrityError):
+            DBTaskResult.objects.create(
+                task_path="", backend_name="default", priority=-101, args_kwargs={}
+            )
+
+        with self.assertRaises(IntegrityError):
+            DBTaskResult.objects.create(
+                task_path="", backend_name="default", priority=101, args_kwargs={}
+            )
+
+        # Django accepts the float, but only stores an int
+        result = DBTaskResult.objects.create(
+            task_path="", backend_name="default", priority=3.1, args_kwargs={}
+        )
+        result.refresh_from_db()
+        self.assertEqual(result.priority, 3)
+
+        DBTaskResult.objects.create(
+            task_path="", backend_name="default", priority=100, args_kwargs={}
+        )
+        DBTaskResult.objects.create(
+            task_path="", backend_name="default", priority=-100, args_kwargs={}
+        )
+        DBTaskResult.objects.create(
+            task_path="", backend_name="default", priority=0, args_kwargs={}
+        )
+
 
 @override_settings(
     TASKS={
@@ -426,6 +455,7 @@ def test_run_after_priority(self) -> None:
         high_priority_result = test_tasks.noop_task.using(priority=10).enqueue()
 
         low_priority_result = test_tasks.noop_task.using(priority=2).enqueue()
+        lower_priority_result = test_tasks.noop_task.using(priority=-2).enqueue()
 
         self.assertEqual(
             [dbt.task_result for dbt in DBTaskResult.objects.all()],
@@ -435,6 +465,7 @@ def test_run_after_priority(self) -> None:
                 low_priority_result,
                 far_future_result,
                 future_result,
+                lower_priority_result,
             ],
         )
 
@@ -443,6 +474,7 @@ def test_run_after_priority(self) -> None:
             [
                 high_priority_result,
                 low_priority_result,
+                lower_priority_result,
             ],
         )
 

tests/tests/test_tasks.py

@@ -20,6 +20,7 @@
     InvalidTaskError,
     ResultDoesNotExist,
 )
+from django_tasks.task import MAX_PRIORITY, MIN_PRIORITY
 from tests import tasks as test_tasks
 
 
@@ -133,9 +134,26 @@ def test_naive_datetime(self) -> None:
 
     def test_invalid_priority(self) -> None:
         with self.assertRaisesMessage(
-            InvalidTaskError, "priority must be zero or greater"
+            InvalidTaskError,
+            f"priority must be a whole number between {MIN_PRIORITY} and {MAX_PRIORITY}",
         ):
-            test_tasks.noop_task.using(priority=-1)
+            test_tasks.noop_task.using(priority=-101)
+
+        with self.assertRaisesMessage(
+            InvalidTaskError,
+            f"priority must be a whole number between {MIN_PRIORITY} and {MAX_PRIORITY}",
+        ):
+            test_tasks.noop_task.using(priority=101)
+
+        with self.assertRaisesMessage(
+            InvalidTaskError,
+            f"priority must be a whole number between {MIN_PRIORITY} and {MAX_PRIORITY}",
+        ):
+            test_tasks.noop_task.using(priority=3.1)  # type:ignore[arg-type]
+
+        test_tasks.noop_task.using(priority=100)
+        test_tasks.noop_task.using(priority=-100)
+        test_tasks.noop_task.using(priority=0)
 
     def test_call_task(self) -> None:
         self.assertEqual(test_tasks.calculate_meaning_of_life.call(), 42)