Skip to content

Commit 0d8fdfa

Browse files
fsavoiakairoaraujo
andauthored
refactoring: update the task response data in the /api/v1/tasks (#518)
* checking the result task status to return an ERRORED state in case of a error, false status * implementing a default value to avoid Exception * adding compability with issue 351 from worker * adding more tests to cover an empty result * * removing message in test_task from details This commit aims to adjust the tests of the get status feature aiming for compatibility with issue #351 from Worker. This issue aims to refactor the task result to facilitate the visualization of the message and error fields, moving from the details field to the data. * * Refactor: add compability with new TaskResult refactored in Worker In discussion with @kairoaraujo, we identified an opportunity to enhance the consistency of Task Results in our codebase. To address this, we open pull request #439 in the Worker repository. This commit introduces these changes to ensure a consistent data structure and aligns with the specified API documentation. --------- Co-authored-by: Kairo Araujo <[email protected]>
1 parent e5747ff commit 0d8fdfa

File tree

3 files changed

+157
-16
lines changed

3 files changed

+157
-16
lines changed

docs/swagger.json

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1528,8 +1528,9 @@
15281528
"title": "Error",
15291529
"description": "If the task status result is `False` shows an error message"
15301530
},
1531-
"any": {
1532-
"title": "Any",
1531+
"details": {
1532+
"type": "object",
1533+
"title": "Details",
15331534
"description": "Any releavant information from task"
15341535
}
15351536
},
@@ -1606,6 +1607,7 @@
16061607
"REJECTED",
16071608
"RETRY",
16081609
"IGNORED",
1610+
"ERRORED",
16091611
"RUNNING"
16101612
],
16111613
"title": "TaskState",
@@ -1624,7 +1626,7 @@
16241626
"$ref": "#/components/schemas/TaskState"
16251627
}
16261628
],
1627-
"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`REVOKED`: Task revoked.\n\n`REJECTED`: Task was rejected (only used in events).\n\n"
1629+
"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"
16281630
},
16291631
"result": {
16301632
"anyOf": [
@@ -1836,11 +1838,17 @@
18361838
"task_id": "33e66671dcc84cdfa2535a1eb030104c",
18371839
"state": "SUCCESS",
18381840
"result": {
1839-
"status": true,
18401841
"task": "add_targets",
1842+
"status": true,
18411843
"last_update": "2023-11-17T09:54:15.762882",
1844+
"message": "Target(s) Added",
18421845
"details": {
1843-
"message": "Target(s) Added"
1846+
"targets": [
1847+
"file1.tar.gz"
1848+
],
1849+
"target_roles": [
1850+
"bins-3"
1851+
]
18441852
}
18451853
}
18461854
},

repository_service_tuf_api/tasks.py

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import enum
77
from datetime import datetime
8-
from typing import Any, Optional, Union
8+
from typing import Any, Dict, Optional, Union
99

1010
from celery import states
1111
from pydantic import BaseModel, Field
@@ -23,6 +23,7 @@ class TaskState(str, enum.Enum):
2323
REJECTED = states.REJECTED
2424
RETRY = states.RETRY
2525
IGNORED = states.IGNORED
26+
ERRORED = "ERRORED"
2627
RUNNING = "RUNNING" # custom state used when a task is RUNNING in RSTUF
2728

2829

@@ -48,7 +49,9 @@ class TaskDetails(BaseModel):
4849
"If the task status result is `False` shows an error message"
4950
)
5051
)
51-
any: Any = Field(description="Any releavant information from task")
52+
details: Optional[Dict[str, Any]] = Field(
53+
description="Any releavant information from task"
54+
)
5255

5356

5457
class TaskResult(BaseModel):
@@ -72,6 +75,8 @@ class TasksData(BaseModel):
7275
"`RUNNING`: Task is running.\n\n"
7376
"`SUCCESS`: Task succeeded.\n\n"
7477
"`FAILURE`: Task failed.\n\n"
78+
"`ERRORED`: Task errored. RSTUF identified an error while "
79+
"processing the task.\n\n"
7580
"`REVOKED`: Task revoked.\n\n"
7681
"`REJECTED`: Task was rejected (only used in events).\n\n"
7782
)
@@ -93,10 +98,14 @@ class Config:
9398
"task_id": "33e66671dcc84cdfa2535a1eb030104c",
9499
"state": TaskState.SUCCESS,
95100
"result": {
96-
"status": True,
97101
"task": TaskName.ADD_TARGETS,
102+
"status": True,
98103
"last_update": "2023-11-17T09:54:15.762882",
99-
"details": {"message": "Target(s) Added"},
104+
"message": "Target(s) Added",
105+
"details": {
106+
"targets": ["file1.tar.gz"],
107+
"target_roles": ["bins-3"],
108+
},
100109
},
101110
},
102111
"message": "Task state.",
@@ -120,12 +129,18 @@ def get(task_id: str) -> Response:
120129
``Response`` as BaseModel from pydantic
121130
"""
122131
task = repository_metadata.AsyncResult(task_id)
132+
133+
task_state = task.state
134+
task_result = task.result
135+
123136
if isinstance(task.result, Exception):
124-
task_result = str(task.result)
125-
else:
126-
task_result = task.result
137+
task_result = {"message": str(task.result)}
138+
elif task_state == TaskState.SUCCESS and not task_result.get(
139+
"status", False
140+
):
141+
task_state = TaskState.ERRORED
127142

128143
return Response(
129-
data=TasksData(task_id=task_id, state=task.state, result=task_result),
144+
data=TasksData(task_id=task_id, state=task_state, result=task_result),
130145
message="Task state.",
131146
)

tests/unit/api/test_tasks.py

Lines changed: 121 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,16 @@ def test_get(self, test_client, monkeypatch):
1717
"status": True,
1818
"task": "add_targets",
1919
"last_update": "2023-11-17T09:54:15.762882",
20-
"details": {"message": "Target(s) Added"},
20+
"message": "Target(s) Added",
21+
"error": None,
22+
"details": {
23+
"targets": [
24+
"file1.tar.gz",
25+
"file2.tar.gz",
26+
"file3.tar.gz",
27+
],
28+
"target_roles": ["bins-3", "bins-2"],
29+
},
2130
},
2231
)
2332
mocked_repository_metadata = pretend.stub(
@@ -38,7 +47,15 @@ def test_get(self, test_client, monkeypatch):
3847
"status": True,
3948
"task": "add_targets",
4049
"last_update": "2023-11-17T09:54:15.762882",
41-
"details": {"message": "Target(s) Added"},
50+
"message": "Target(s) Added",
51+
"details": {
52+
"targets": [
53+
"file1.tar.gz",
54+
"file2.tar.gz",
55+
"file3.tar.gz",
56+
],
57+
"target_roles": ["bins-3", "bins-2"],
58+
},
4259
},
4360
},
4461
"message": "Task state.",
@@ -65,7 +82,108 @@ def test_get_result_is_exception(self, test_client, monkeypatch):
6582
"data": {
6683
"task_id": "test_id",
6784
"state": "SUCCESS",
68-
"result": "Failed to load",
85+
"result": {"message": "Failed to load"},
86+
},
87+
"message": "Task state.",
88+
}
89+
assert mocked_repository_metadata.AsyncResult.calls == [
90+
pretend.call("test_id")
91+
]
92+
93+
def test_get_result_is_errored(self, test_client, monkeypatch):
94+
mocked_task_result = pretend.stub(
95+
state="SUCCESS",
96+
result={
97+
"status": False,
98+
"task": "sign_metadata",
99+
"last_update": "2023-11-17T09:54:15.762882",
100+
"message": "Signature Failed",
101+
"error": "No signatures pending for root",
102+
"details": None,
103+
},
104+
)
105+
mocked_repository_metadata = pretend.stub(
106+
AsyncResult=pretend.call_recorder(lambda t: mocked_task_result)
107+
)
108+
monkeypatch.setattr(
109+
"repository_service_tuf_api.tasks.repository_metadata",
110+
mocked_repository_metadata,
111+
)
112+
113+
test_response = test_client.get(f"{TASK_URL}?task_id=test_id")
114+
assert test_response.status_code == status.HTTP_200_OK
115+
116+
assert test_response.json() == {
117+
"data": {
118+
"task_id": "test_id",
119+
"state": "ERRORED",
120+
"result": {
121+
"status": False,
122+
"task": "sign_metadata",
123+
"last_update": "2023-11-17T09:54:15.762882",
124+
"message": "Signature Failed",
125+
"error": "No signatures pending for root",
126+
},
127+
},
128+
"message": "Task state.",
129+
}
130+
assert mocked_repository_metadata.AsyncResult.calls == [
131+
pretend.call("test_id")
132+
]
133+
134+
def test_get_result_success_with_empty_result(
135+
self, test_client, monkeypatch
136+
):
137+
mocked_task_result = pretend.stub(
138+
state="SUCCESS",
139+
result={},
140+
)
141+
mocked_repository_metadata = pretend.stub(
142+
AsyncResult=pretend.call_recorder(lambda t: mocked_task_result)
143+
)
144+
monkeypatch.setattr(
145+
"repository_service_tuf_api.tasks.repository_metadata",
146+
mocked_repository_metadata,
147+
)
148+
149+
test_response = test_client.get(f"{TASK_URL}?task_id=test_id")
150+
assert test_response.status_code == status.HTTP_200_OK
151+
152+
assert test_response.json() == {
153+
"data": {
154+
"task_id": "test_id",
155+
"state": "ERRORED",
156+
"result": {},
157+
},
158+
"message": "Task state.",
159+
}
160+
assert mocked_repository_metadata.AsyncResult.calls == [
161+
pretend.call("test_id")
162+
]
163+
164+
def test_get_result_failure_with_empty_result(
165+
self, test_client, monkeypatch
166+
):
167+
mocked_task_result = pretend.stub(
168+
state="FAILURE",
169+
result={},
170+
)
171+
mocked_repository_metadata = pretend.stub(
172+
AsyncResult=pretend.call_recorder(lambda t: mocked_task_result)
173+
)
174+
monkeypatch.setattr(
175+
"repository_service_tuf_api.tasks.repository_metadata",
176+
mocked_repository_metadata,
177+
)
178+
179+
test_response = test_client.get(f"{TASK_URL}?task_id=test_id")
180+
assert test_response.status_code == status.HTTP_200_OK
181+
182+
assert test_response.json() == {
183+
"data": {
184+
"task_id": "test_id",
185+
"state": "FAILURE",
186+
"result": {},
69187
},
70188
"message": "Task state.",
71189
}

0 commit comments

Comments
 (0)