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

FEAT: Enhance API Deployment Validation to Return 4xx Instead of 500 for Errors #890

Merged
merged 22 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
e860ea6
Added validation to execution_id and returning proper error status fo…
pk-zipstack Dec 6, 2024
c9aaac4
Added validation for executions in serializers
pk-zipstack Dec 10, 2024
b25a020
Merge branch 'main' into feat/improve-api-deployment-valdiation
pk-zipstack Dec 10, 2024
a2e5d92
Merge branch 'main' into feat/improve-api-deployment-valdiation
pk-zipstack Dec 11, 2024
69c186b
Merge branch 'main' into feat/improve-api-deployment-valdiation
pk-zipstack Dec 11, 2024
baaa0e7
Added separate serialzers for GET api deployments
pk-zipstack Dec 11, 2024
d9b9a80
Modified workflow_helper.py
pk-zipstack Dec 11, 2024
ec0594f
Modified workflow_helper.py
pk-zipstack Dec 11, 2024
dd3d044
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 11, 2024
3268cbc
Update workflow_helper.py
pk-zipstack Dec 11, 2024
48c58a0
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 11, 2024
d13ffb4
Update backend/api_v2/serializers.py
pk-zipstack Dec 12, 2024
6e86a3d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 12, 2024
98e6e86
Removed unnecesary try block in api_deployment_views.py
pk-zipstack Dec 12, 2024
27bf54f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 12, 2024
4df8e92
Update backend/workflow_manager/workflow_v2/workflow_helper.py
pk-zipstack Dec 16, 2024
338f8d4
Moved checking for existence of execution to serializers
pk-zipstack Dec 16, 2024
99544c5
Removed unused import in workflow_helper.py
pk-zipstack Dec 16, 2024
9a671bb
Merge branch 'main' into feat/improve-api-deployment-valdiation
pk-zipstack Dec 16, 2024
d6e03d6
Update backend/api_v2/serializers.py
pk-zipstack Dec 16, 2024
dddf46e
Merge branch 'main' into feat/improve-api-deployment-valdiation
pk-zipstack Dec 16, 2024
8849688
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 16, 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
26 changes: 16 additions & 10 deletions backend/api_v2/api_deployment_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
APIDeploymentListSerializer,
APIDeploymentSerializer,
DeploymentResponseSerializer,
ExecutionQuerySerializer,
ExecutionRequestSerializer,
)
from django.db.models import QuerySet
Expand Down Expand Up @@ -73,21 +74,26 @@ def post(
def get(
self, request: Request, org_name: str, api_name: str, api: APIDeployment
) -> Response:
execution_id = request.query_params.get("execution_id")
include_metadata = (
request.query_params.get(ApiExecution.INCLUDE_METADATA, "false").lower()
== "true"
)
if not execution_id:
raise InvalidAPIRequest("execution_id shouldn't be empty")
response: ExecutionResponse = DeploymentHelper.get_execution_status(
execution_id=execution_id
)
serializer = ExecutionQuerySerializer(data=request.query_params)
serializer.is_valid(raise_exception=True)

execution_id = serializer.validated_data.get(ApiExecution.EXECUTION_ID)
include_metadata = serializer.validated_data.get(ApiExecution.INCLUDE_METADATA)
chandrasekharan-zipstack marked this conversation as resolved.
Show resolved Hide resolved

try:
response: ExecutionResponse = DeploymentHelper.get_execution_status(
execution_id
)
except InvalidAPIRequest as e:
chandrasekharan-zipstack marked this conversation as resolved.
Show resolved Hide resolved
logger.error(f"Invalid execution_id: {execution_id}. Error: {e}")
return Response({"error": str(e)}, status=status.HTTP_404_NOT_FOUND)

response_status = status.HTTP_422_UNPROCESSABLE_ENTITY
if response.execution_status == CeleryTaskState.COMPLETED.value:
response_status = status.HTTP_200_OK
if not include_metadata:
response.remove_result_metadata_keys()

return Response(
data={
"status": response.execution_status,
Expand Down
1 change: 1 addition & 0 deletions backend/api_v2/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ class ApiExecution:
TIMEOUT_FORM_DATA: str = "timeout"
INCLUDE_METADATA: str = "include_metadata"
USE_FILE_HISTORY: str = "use_file_history" # Undocumented parameter
EXECUTION_ID: str = "execution_id"
15 changes: 15 additions & 0 deletions backend/api_v2/serializers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import uuid
from collections import OrderedDict
from typing import Any, Union

Expand Down Expand Up @@ -115,6 +116,20 @@ class ExecutionRequestSerializer(Serializer):
use_file_history = BooleanField(default=False)


class ExecutionQuerySerializer(Serializer):
execution_id = CharField(required=True)
include_metadata = BooleanField(default=False)

def validate_execution_id(self, value):
"""Trim spaces and validate execution_id as UUID."""
value = value.strip()
try:
uuid.UUID(value)
except ValueError:
raise ValidationError("Invalid execution_id. Must be a valid UUID.")
pk-zipstack marked this conversation as resolved.
Show resolved Hide resolved
return value


class APIDeploymentListSerializer(ModelSerializer):
workflow_name = CharField(source="workflow.workflow_name", read_only=True)

Expand Down
5 changes: 5 additions & 0 deletions backend/workflow_manager/workflow_v2/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ class WorkflowDoesNotExistError(APIException):
default_detail = "Workflow does not exist"


class ExecutionDoesNotExistError(APIException):
status_code = 404
default_detail = "Execution does not exist."


class TaskDoesNotExistError(APIException):
status_code = 404
default_detail = "Task does not exist"
Expand Down
18 changes: 13 additions & 5 deletions backend/workflow_manager/workflow_v2/workflow_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
from workflow_manager.workflow_v2.dto import AsyncResultData, ExecutionResponse
from workflow_manager.workflow_v2.enums import ExecutionStatus, SchemaEntity, SchemaType
from workflow_manager.workflow_v2.exceptions import (
ExecutionDoesNotExistError,
InvalidRequest,
TaskDoesNotExistError,
WorkflowDoesNotExistError,
Expand Down Expand Up @@ -369,18 +370,25 @@ def get_status_of_async_task(

Raises:
TaskDoesNotExistError: Not found exception
ExecutionDoesNotExistError: If execution is not found

Returns:
ExecutionResponse: _description_
"""
execution = WorkflowExecution.objects.get(id=execution_id)

if not execution.task_id:
raise TaskDoesNotExistError()
try:
execution = WorkflowExecution.objects.get(id=execution_id)
if not execution.task_id:
raise TaskDoesNotExistError(
f"No task ID found for execution: {execution_id}"
)
except WorkflowExecution.DoesNotExist:
raise ExecutionDoesNotExistError(
f"No execution found with id: {execution_id}"
)
pk-zipstack marked this conversation as resolved.
Show resolved Hide resolved

result = AsyncResult(str(execution.task_id))

task = AsyncResultData(async_result=result)

return ExecutionResponse(
execution.workflow_id,
execution_id,
Expand Down
Loading