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.

Sign up for GitHub

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

Jump to bottom

fix: API documentation for Task Result type #524

Merged
merged 4 commits into from
Feb 9, 2024
Merged

fix: API documentation for Task Result type #524

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9d7310c
fix: API docs Task and improve consistency
kairoaraujo Jan 30, 2024
cb5bf33
docs: Include descriptions in the Swagger (Model)
kairoaraujo Feb 1, 2024
ddd900a
fixup! fix: API docs Task and improve consistency
kairoaraujo Feb 7, 2024
c55f15c
fixup! fixup! fix: API docs Task and improve consistency
kairoaraujo Feb 7, 2024
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
54 changes: 14 additions & 40 deletions docs/swagger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1516,30 +1516,6 @@
],
"title": "TargetsInfo"
},
"TaskDetails": {
"properties": {
"message": {
"type": "string",
"title": "Message",
"description": "Result detail description"
},
"error": {
"type": "string",
"title": "Error",
"description": "If the task status result is `False` shows an error message"
},
"details": {
"type": "object",
"title": "Details",
"description": "Any releavant information from task"
}
},
"type": "object",
"required": [
"message"
],
"title": "TaskDetails"
},
"TaskName": {
"type": "string",
"enum": [
Expand All @@ -1557,6 +1533,16 @@
},
"TaskResult": {
"properties": {
"message": {
"type": "string",
"title": "Message",
"description": "Result detail description"
},
"error": {
"type": "string",
"title": "Error",
"description": "If the task status result is `False` shows an error message"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this API user facing documentation? They don't know about task status, right?

},
"status": {
"type": "boolean",
"title": "Status",
Expand All @@ -1577,22 +1563,12 @@
"description": "Last time task was updated"
},
"details": {
"allOf": [
{
"$ref": "#/components/schemas/TaskDetails"
}
],
"type": "object",
"title": "Details",
"description": "Relevant result details"
}
},
"type": "object",
"required": [
"status",
"task",
"last_update",
"details"
],
"title": "TaskResult"
},
"TaskState": {
Expand Down Expand Up @@ -1626,17 +1602,16 @@
"$ref": "#/components/schemas/TaskState"
}
],
"description": "The Celery task state. Note: It isn't the task result status.\n\n`PENDING`: Task state is unknown (assumed pending since you know the id).\n\n`RECEIVED`: Task received by a worker (only used in events).\n\n`STARTED`: Task started by a worker.\n\n`RUNNING`: Task is running.\n\n`SUCCESS`: Task succeeded.\n\n`FAILURE`: Task failed.\n\n`ERRORED`: Task errored. RSTUF identified an error while processing the task.\n\n`REVOKED`: Task revoked.\n\n`REJECTED`: Task was rejected (only used in events).\n\n"
"description": "The Celery task state. Note: It isn't the task result status.\n\n`PENDING`: Task state is unknown (assumed pending since you know the id).\n\n`RECEIVED`: Task received by a RSTUF Worker (only used in events).\n\n`SUCCESS`: Task succeeded.\n\n`STARTED`: Task started by a RSTUF Worker.\n\n`RUNNING`: Task is running on RSTUF Worker.\n\n`FAILURE`: Task failed (unexpected).\n\n`REVOKED`: Task revoked.\n\n`RETRY`: Task is waiting for retry.\n\n`ERRORED`: Task errored. RSTUF identified an error while processing the task.\n\n`REJECTED`: Task was rejected (only used in events).\n\n`IGNORED`: Task was ignored."
},
"result": {
"anyOf": [
{},
"allOf": [
{
"$ref": "#/components/schemas/TaskResult"
}
],
"title": "Result",
"description": "Task result details (state `SUCCESS` uses schema `TaskResult)"
"description": "Task result if available."
}
},
"type": "object",
Expand Down Expand Up @@ -1839,7 +1814,6 @@
"state": "SUCCESS",
"result": {
"task": "add_targets",
"status": true,
"last_update": "2023-11-17T09:54:15.762882",
"message": "Target(s) Added",
"details": {
Expand Down
56 changes: 31 additions & 25 deletions repository_service_tuf_api/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import enum
from datetime import datetime
from typing import Any, Dict, Optional, Union
from typing import Any, Dict, Optional

from celery import states
from pydantic import BaseModel, Field
Expand Down Expand Up @@ -42,25 +42,24 @@ class GetParameters(BaseModel):
task_id: str


class TaskDetails(BaseModel):
message: str = Field(description="Result detail description")
class TaskResult(BaseModel):
message: Optional[str] = Field(description="Result detail description")
error: Optional[str] = Field(
description=(
"If the task status result is `False` shows an error message"
)
)
details: Optional[Dict[str, Any]] = Field(
description="Any releavant information from task"
status: Optional[bool] = Field(
description="Task result status. `True` Success | `False` Failure",
exclude=True,
)


class TaskResult(BaseModel):
status: bool = Field(
description="Task result status. `True` Success | `False` Failure"
task: Optional[TaskName] = Field(description="Task name by worker")
last_update: Optional[datetime] = Field(
description="Last time task was updated"
)
details: Optional[Dict[str, Any]] = Field(
description="Relevant result details"
)
task: TaskName = Field(description="Task name by worker")
last_update: datetime = Field(description="Last time task was updated")
details: TaskDetails = Field(description="Relevant result details")


class TasksData(BaseModel):
Expand All @@ -70,21 +69,22 @@ class TasksData(BaseModel):
"The Celery task state. Note: It isn't the task result status.\n\n"
"`PENDING`: Task state is unknown (assumed pending since you know "
"the id).\n\n"
"`RECEIVED`: Task received by a worker (only used in events).\n\n"
"`STARTED`: Task started by a worker.\n\n"
"`RUNNING`: Task is running.\n\n"
"`RECEIVED`: Task received by a RSTUF Worker (only used in "
"events).\n\n"
"`SUCCESS`: Task succeeded.\n\n"
"`FAILURE`: Task failed.\n\n"
"`STARTED`: Task started by a RSTUF Worker.\n\n"
"`RUNNING`: Task is running on RSTUF Worker.\n\n"
"`FAILURE`: Task failed (unexpected).\n\n"
"`REVOKED`: Task revoked.\n\n"
"`RETRY`: Task is waiting for retry.\n\n"
"`ERRORED`: Task errored. RSTUF identified an error while "
"processing the task.\n\n"
"`REVOKED`: Task revoked.\n\n"
"`REJECTED`: Task was rejected (only used in events).\n\n"
"`IGNORED`: Task was ignored."
)
)
result: Optional[Union[Any, TaskResult]] = Field(
description=(
"Task result details (state `SUCCESS` uses schema `TaskResult)"
)
result: Optional[TaskResult] = Field(
description=("Task result if available.")
)


Expand All @@ -99,7 +99,6 @@ class Config:
"state": TaskState.SUCCESS,
"result": {
"task": TaskName.ADD_TARGETS,
"status": True,
"last_update": "2023-11-17T09:54:15.762882",
"message": "Target(s) Added",
"details": {
Expand Down Expand Up @@ -133,9 +132,16 @@ def get(task_id: str) -> Response:
task_state = task.state
task_result = task.result

# Celery FAILURE task, we include the task result (exception) as an error
# and default message as critical failure executing the task.
if isinstance(task.result, Exception):
task_result = {"message": str(task.result)}
elif task_state == TaskState.SUCCESS and not task_result.get(
task_result = {
"message": f"Task failure: {str(task.result)}",
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: IMO you can omit "Task failure: ". The info is redundant with the state.

}

# If the task state is SUCCESS and the task result is False we considere
# it an errored task.
if task_state == TaskState.SUCCESS and not task_result.get(
"status", False
):
task_state = TaskState.ERRORED
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/api/test_tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ def test_get(self, test_client, monkeypatch):
"task_id": "test_id",
"state": "SUCCESS",
"result": {
"status": True,
"task": "add_targets",
"last_update": "2023-11-17T09:54:15.762882",
"message": "Target(s) Added",
Expand All @@ -66,7 +65,7 @@ def test_get(self, test_client, monkeypatch):

def test_get_result_is_exception(self, test_client, monkeypatch):
mocked_task_result = pretend.stub(
state="SUCCESS", result=ValueError("Failed to load")
state="FAILURE", result=ValueError("Failed to load")
)
mocked_repository_metadata = pretend.stub(
AsyncResult=pretend.call_recorder(lambda t: mocked_task_result)
Expand All @@ -81,8 +80,10 @@ def test_get_result_is_exception(self, test_client, monkeypatch):
assert test_response.json() == {
"data": {
"task_id": "test_id",
"state": "SUCCESS",
"result": {"message": "Failed to load"},
"state": "FAILURE",
"result": {
"message": "Task failure: Failed to load",
},
},
"message": "Task state.",
}
Expand Down Expand Up @@ -118,7 +119,6 @@ def test_get_result_is_errored(self, test_client, monkeypatch):
"task_id": "test_id",
"state": "ERRORED",
"result": {
"status": False,
"task": "sign_metadata",
"last_update": "2023-11-17T09:54:15.762882",
"message": "Signature Failed",
Expand Down
Loading