Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for task id as valid UUID #3744

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions rocky/rocky/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,15 +322,19 @@ def list_tasks(self, **kwargs) -> PaginatedTasksResponse:
filter_key = "filters"
params = {k: v for k, v in kwargs.items() if v is not None if k != filter_key} # filter Nones from kwargs
endpoint = "/tasks"
res = self._client.post(endpoint, params=params, json=kwargs.get(filter_key, None))
res = self._client.post(endpoint, params=params, json=kwargs.get(filter_key))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is automatically removed and is not part of this PR. None is already a default return value.

return PaginatedTasksResponse.model_validate_json(res.content)
except ValidationError:
raise SchedulerValidationError(extra_message=_("Task list: "))
except ConnectError:
raise SchedulerConnectError(extra_message=_("Task list: "))

def get_task_details(self, task_id: str) -> Task:
return Task.model_validate_json(self._get(f"/tasks/{task_id}", "content"))
try:
task_id = str(uuid.UUID(task_id))
return Task.model_validate_json(self._get(f"/tasks/{task_id}", "content"))
except ValueError:
raise SchedulerTaskNotFound()

def push_task(self, item: Task) -> None:
try:
Expand Down
56 changes: 28 additions & 28 deletions rocky/rocky/views/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from typing import Any

from django.contrib import messages
from django.http import JsonResponse
from django.http import Http404, JsonResponse
from django.utils.translation import gettext_lazy as _
from katalogus.client import Boefje, Normalizer
from reports.forms import (
Expand Down Expand Up @@ -112,13 +112,15 @@ def get_report_child_name_form(self):
return self.report_child_name_form()

def get_task_details(self, task_id: str) -> Task | None:
task = self.scheduler_client.get_task_details(task_id)

if task.organization_id() != self.organization.code:
messages.error(self.request, SchedulerTaskNotFound.message)
return None
try:
task = self.scheduler_client.get_task_details(task_id)
if task.organization_id() != self.organization.code:
messages.error(self.request, SchedulerTaskNotFound.message)
return None
Rieven marked this conversation as resolved.
Show resolved Hide resolved

return task
return task
except SchedulerTaskNotFound:
raise Http404()

def create_report_schedule(self, report_recipe: ReportRecipe, deadline_at: str) -> ScheduleResponse | None:
try:
Expand Down Expand Up @@ -169,19 +171,16 @@ def get_output_oois(self, task):
return []

def get_json_task_details(self) -> JsonResponse | None:
try:
task = self.get_task_details(self.kwargs["task_id"])
if task:
return JsonResponse(
{
"oois": self.get_output_oois(task),
"valid_time": task.data.raw_data.boefje_meta.ended_at.strftime("%Y-%m-%dT%H:%M:%S"),
},
safe=False,
)
return task
except SchedulerError as error:
return messages.error(self.request, error.message)
task = self.get_task_details(self.kwargs["task_id"])
Rieven marked this conversation as resolved.
Show resolved Hide resolved
if task:
return JsonResponse(
{
"oois": self.get_output_oois(task),
"valid_time": task.data.raw_data.boefje_meta.ended_at.strftime("%Y-%m-%dT%H:%M:%S"),
},
safe=False,
)
return task

def get_schedule_with_filters(self, filters: dict[str, list[dict[str, str]]]) -> ScheduleResponse:
try:
Expand Down Expand Up @@ -210,18 +209,19 @@ def schedule_task(self, task: Task) -> None:
# task info from the scheduler. Task data should be available from the context
# from which the task is created.
def reschedule_task(self, task_id: str) -> None:
task = self.scheduler_client.get_task_details(task_id)
task = self.get_task_details(task_id)

if task.organization_id() != self.organization.code:
messages.error(self.request, SchedulerTaskNotFound.message)
return
if task is not None:
if task.organization_id() != self.organization.code:
messages.error(self.request, SchedulerTaskNotFound.message)
return
Rieven marked this conversation as resolved.
Show resolved Hide resolved

new_id = uuid.uuid4()
task.data.id = new_id
new_id = uuid.uuid4()
task.data.id = new_id

new_task = Task(id=new_id, scheduler_id=task.scheduler_id, priority=1, data=task.data)
new_task = Task(id=new_id, scheduler_id=task.scheduler_id, priority=1, data=task.data)

self.schedule_task(new_task)
self.schedule_task(new_task)

def run_normalizer(self, katalogus_normalizer: Normalizer, raw_data: RawData) -> None:
try:
Expand Down
1 change: 1 addition & 0 deletions rocky/rocky/views/task_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ def get_context_data(self, **kwargs):
class NormalizerTaskJSONView(TaskDetailView):
task_type = "normalizer"
plugin_type = "normalizer"
template_name = "tasks/normalizer_task_detail.html"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My test failed on this part, because it was missing the template name


def get(self, request, *args, **kwargs) -> JsonResponse | HttpResponse:
task = self.get_json_task_details()
Expand Down
13 changes: 4 additions & 9 deletions rocky/rocky/views/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,10 @@ def get_queryset(self):
return self.get_task_list()

def post(self, request, *args, **kwargs):
try:
if self.action == self.RESCHEDULE_TASK:
task_id = self.request.POST.get("task_id", "")
self.reschedule_task(task_id)
except HTTPError as exc:
message = f"HTTP error for {exc.request.url} - {exc}"
messages.error(request, message)
except SchedulerError as error:
messages.error(request, error.message)
if self.action == self.RESCHEDULE_TASK:
task_id = self.request.POST.get("task_id", "")
self.reschedule_task(task_id)

return super().post(request, *args, **kwargs)

def get_context_data(self, **kwargs):
Expand Down
30 changes: 29 additions & 1 deletion rocky/tests/scheduler/test_scheduler_errors.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
from rocky.scheduler import SchedulerConnectError, SchedulerTooManyRequestError, SchedulerValidationError
import pytest
from django.http import Http404

from rocky.scheduler import (
SchedulerConnectError,
SchedulerTaskNotFound,
SchedulerTooManyRequestError,
SchedulerValidationError,
)
from rocky.views.task_detail import NormalizerTaskJSONView
from rocky.views.tasks import BoefjesTaskListView
from tests.conftest import setup_request

Expand Down Expand Up @@ -37,3 +46,22 @@ def test_tasks_view_too_many_requests_error(rf, client_member, mock_scheduler):
list(request._messages)[0].message
== "Scheduler is receiving too many requests. Increase SCHEDULER_PQ_MAXSIZE or wait for task to finish."
)


def test_get_task_details_json_bad_task_id(rf, client_member, mock_scheduler):
mock_scheduler.get_task_details.side_effect = SchedulerTaskNotFound
request = setup_request(rf.get("normalizer_task_view"), client_member.user)

with pytest.raises(Http404):
NormalizerTaskJSONView.as_view()(request, organization_code=client_member.organization.code, task_id="/delete")


def test_reschedule_task_bad_task_id(rf, client_member, mock_bytes_client, mock_scheduler):
mock_scheduler.get_task_details.side_effect = SchedulerTaskNotFound

request = setup_request(
rf.post("task_list", {"action": "reschedule_task", "task_id": "/delete"}), client_member.user
)

with pytest.raises(Http404):
BoefjesTaskListView.as_view()(request, organization_code=client_member.organization.code)