Skip to content

Commit

Permalink
feat: allow custom HTTP status codes when using DiscardTaskException
Browse files Browse the repository at this point in the history
To prevent a Google Cloud Task from being retried, it is necessary to return a status code in the 200-299 range. The mechanism django_cloud_tasks currently offers for this is raising `DiscardTaskException`, but in this case the status code will always be HTTP 202 (Accepted). When we want to discard a task due to an unrecoverable error, this HTTP status code offers no good semantics. Also, from a monitoring perspective, simply discarding a task (perhaps it is no longer needed - for example: attempting to delete something that has already been deleted) and discarding a task due to an unrecoverable error (for example: the task input is invalid) are two different things, but we have no means to differentiate them if the status code is always the same.

This PR adds a bit of flexibility, allowing DiscardTaskException to receive an HTTP status code / HTTP status reason phrase as either constructor arguments, or by subclassing it and overriding default_http_status_code / default_http_status_reason.

This PR won't add a built-in "UnrecoverableTaskException" base class because there is no HTTP 2xx status code (even when considering augmented standards) to reflect this scenario, so we will leave it up to each project that uses django-cloud-tasks to configure this setup, as it will be project-specific by definition.
  • Loading branch information
rodrigoalmeidaee committed Aug 7, 2024
1 parent 022f343 commit b8f2868
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 12 deletions.
9 changes: 8 additions & 1 deletion django_cloud_tasks/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,11 @@ def __init__(self, name: str):
super().__init__(message)


class DiscardTaskException(Exception): ...
class DiscardTaskException(Exception):
default_http_status_code: int = 202
default_http_status_reason: str | None = None # only needed for custom HTTP status codes

def __init__(self, *args, http_status_code: int | None = None, http_status_reason: str | None = None, **kwargs):
super().__init__(*args, **kwargs)
self.http_status_code = http_status_code or self.default_http_status_code
self.http_status_reason = http_status_reason or self.default_http_status_reason
12 changes: 8 additions & 4 deletions django_cloud_tasks/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,21 @@ def post(self, request, task_name, *args, **kwargs):
output = self.execute_task(task_class=task_class, task_metadata=task_metadata, task_kwargs=task_kwargs)
status = "executed"
status_code = 200
except exceptions.DiscardTaskException:
status_reason = None
except exceptions.DiscardTaskException as e:
output = None
status = "discarded"
status_code = 202
status_code = e.http_status_code
status_reason = e.http_status_reason

data = {"result": output, "status": status}
try:
return JsonResponse(status=status_code, data=data)
return JsonResponse(status=status_code, reason=status_reason, data=data)
except TypeError:
logger.warning(f"Unable to serialize task output from {request.path}: {str(output)}")
return JsonResponse(status=status_code, data={"result": str(output), "status": "executed"})
return JsonResponse(
status=status_code, reason=status_reason, data={"result": str(output), "status": "executed"}
)

def get_task(self, name: str) -> Type[Task]:
app = apps.get_app_config("django_cloud_tasks")
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "django-google-cloud-tasks"
version = "2.16.4"
version = "2.17.0"
description = "Async Tasks with HTTP endpoints"
authors = ["Joao Daher <[email protected]>"]
packages = [
Expand Down
44 changes: 44 additions & 0 deletions sample_project/sample_app/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from django.db.models import Model

from django_cloud_tasks.tasks import PeriodicTask, RoutineTask, SubscriberTask, Task, ModelPublisherTask, TaskMetadata
from django_cloud_tasks.exceptions import DiscardTaskException


class BaseAbstractTask(Task, abc.ABC):
Expand Down Expand Up @@ -99,6 +100,49 @@ def build_message_attributes(cls, obj: Model, event: str, **kwargs) -> dict[str,
return {"any-custom-attribute": "yay!", "event": event}


class FindPrimeNumbersTask(Task):
storage: list[int] = []

@classmethod
def reset(cls):
cls.storage = []

def run(self, quantity):
if not isinstance(quantity, int):
raise DiscardTaskException(
"Can't find a non-integer amount of prime numbers",
http_status_code=299,
http_status_reason="Unretriable failure",
)

if len(self.storage) >= quantity:
raise DiscardTaskException("Nothing to do here")

return self._find_primes(quantity)

@classmethod
def _find_primes(cls, quantity: int) -> list[int]:
if not cls.storage:
cls.storage = [2]

while len(cls.storage) < quantity:
cls.storage.append(cls._find_next_prime(cls.storage[-1] + 1))

return cls.storage

@classmethod
def _find_next_prime(cls, starting_number: int) -> int:
candidate = starting_number

while True:
for prime in cls.storage:
if candidate % prime == 0:
candidate += 1
break
else:
return candidate


class DummyRoutineTask(RoutineTask):
def run(self, **kwargs): ...

Expand Down
14 changes: 8 additions & 6 deletions sample_project/sample_app/tests/tests_tasks/tests_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,18 @@ def app_config(self):
def test_registered_tasks(self):
expected_tasks = {
"CalculatePriceTask",
"DummyRoutineTask",
"ExposeCustomHeadersTask",
"FailMiserablyTask",
"FindPrimeNumbersTask",
"NonCompliantTask",
"OneBigDedicatedTask",
"ParentCallingChildTask",
"PublishPersonTask",
"RoutineExecutorTask",
"RoutineReverterTask",
"SayHelloTask",
"SayHelloWithParamsTask",
"DummyRoutineTask",
"RoutineReverterTask",
"ParentCallingChildTask",
"ExposeCustomHeadersTask",
"PublishPersonTask",
"NonCompliantTask",
}
self.assertEqual(expected_tasks, set(self.app_config.on_demand_tasks))

Expand Down Expand Up @@ -89,6 +90,7 @@ def test_get_tasks(self):
tasks.SayHelloTask,
tasks.SayHelloWithParamsTask,
tasks.PublishPersonTask,
tasks.FindPrimeNumbersTask,
tasks.DummyRoutineTask,
another_app_tasks.deep_down_tasks.one_dedicated_task.OneBigDedicatedTask,
another_app_tasks.deep_down_tasks.one_dedicated_task.NonCompliantTask,
Expand Down
31 changes: 31 additions & 0 deletions sample_project/sample_app/tests/tests_views/tests_task_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from another_app.tasks.deep_down_tasks.one_dedicated_task import NonCompliantTask
from django_cloud_tasks.tasks import TaskMetadata
from sample_app.tests.tests_base_tasks import AuthenticationMixin
from sample_app.tasks import FindPrimeNumbersTask


class TaskViewTest(AuthenticationMixin):
Expand Down Expand Up @@ -116,3 +117,33 @@ def test_non_compliant_task_called(self):
response = self.client.post(path=url, data=data, content_type="application/json")
self.assertEqual(200, response.status_code)
self.assertEqual({"result": ANY, "status": "executed"}, response.json())


class TaskDiscardingTest(AuthenticationMixin):
url = "/tasks/FindPrimeNumbersTask"

def setUp(self):
super().setUp()
FindPrimeNumbersTask.reset()

def call_task(self, data):
return self.client.post(path=self.url, data=data, content_type="application/json")

def test_when_task_is_not_discarded(self):
response = self.call_task(data={"quantity": 3})
self.assertEqual(200, response.status_code)
self.assertEqual({"result": [2, 3, 5], "status": "executed"}, response.json())

def test_when_task_is_discarded_due_to_no_longer_being_needed(self):
self.call_task(data={"quantity": 3})

response = self.call_task(data={"quantity": 3})
self.assertEqual(202, response.status_code)
self.assertEqual("Accepted", response.reason_phrase)
self.assertEqual({"result": None, "status": "discarded"}, response.json())

def test_when_task_is_discarded_due_to_permanent_error(self):
response = self.call_task(data={"quantity": "not-a-number"})
self.assertEqual(299, response.status_code)
self.assertEqual("Unretriable failure", response.reason_phrase)
self.assertEqual({"result": None, "status": "discarded"}, response.json())

0 comments on commit b8f2868

Please sign in to comment.