From fdb5a0cb5e42649f0f36a1efc14cd2de120a8c5f Mon Sep 17 00:00:00 2001 From: Praveen Kumar Date: Tue, 17 Dec 2024 18:23:03 +0530 Subject: [PATCH] FEAT: Enhance API Deployment Validation to Return 4xx Instead of 500 for Errors (#890) * Added validation to execution_id and returning proper error status for execution requests * Added validation for executions in serializers * Merge branch 'main' into feat/improve-api-deployment-valdiation * Merge branch 'main' into feat/improve-api-deployment-valdiation * Added separate serialzers for GET api deployments * Modified workflow_helper.py * Modified workflow_helper.py * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update workflow_helper.py Signed-off-by: Praveen Kumar * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update backend/api_v2/serializers.py Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com> Signed-off-by: Praveen Kumar * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Removed unnecesary try block in api_deployment_views.py Signed-off-by: Praveen Kumar * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update backend/workflow_manager/workflow_v2/workflow_helper.py Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com> Signed-off-by: Praveen Kumar * Moved checking for existence of execution to serializers * Removed unused import in workflow_helper.py * Update backend/api_v2/serializers.py Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com> Signed-off-by: Praveen Kumar * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Signed-off-by: Praveen Kumar Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com> --- backend/api_v2/api_deployment_views.py | 19 +++++++----- backend/api_v2/constants.py | 1 + backend/api_v2/serializers.py | 29 +++++++++++++++++++ .../workflow_v2/exceptions.py | 5 ++++ .../workflow_v2/workflow_helper.py | 7 +++-- 5 files changed, 50 insertions(+), 11 deletions(-) diff --git a/backend/api_v2/api_deployment_views.py b/backend/api_v2/api_deployment_views.py index fddf8d536..017ab4888 100644 --- a/backend/api_v2/api_deployment_views.py +++ b/backend/api_v2/api_deployment_views.py @@ -11,6 +11,7 @@ APIDeploymentListSerializer, APIDeploymentSerializer, DeploymentResponseSerializer, + ExecutionQuerySerializer, ExecutionRequestSerializer, ) from django.db.models import QuerySet @@ -73,16 +74,18 @@ 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") + 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) + + # Fetch execution status response: ExecutionResponse = DeploymentHelper.get_execution_status( - execution_id=execution_id + execution_id ) + + # Determine response status response_status = status.HTTP_422_UNPROCESSABLE_ENTITY if response.execution_status == CeleryTaskState.COMPLETED.value: response_status = status.HTTP_200_OK diff --git a/backend/api_v2/constants.py b/backend/api_v2/constants.py index 2de41efb2..7cdb3c8ba 100644 --- a/backend/api_v2/constants.py +++ b/backend/api_v2/constants.py @@ -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" diff --git a/backend/api_v2/serializers.py b/backend/api_v2/serializers.py index 655478e4b..8bcca93ae 100644 --- a/backend/api_v2/serializers.py +++ b/backend/api_v2/serializers.py @@ -1,3 +1,4 @@ +import uuid from collections import OrderedDict from typing import Any, Union @@ -15,6 +16,8 @@ ValidationError, ) from utils.serializer.integrity_error_mixin import IntegrityErrorMixin +from workflow_manager.workflow_v2.exceptions import ExecutionDoesNotExistError +from workflow_manager.workflow_v2.models.execution import WorkflowExecution from backend.serializers import AuditSerializer @@ -115,6 +118,32 @@ 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, validate UUID format, and check if execution_id exists.""" + value = value.strip() + + # Validate UUID format + try: + uuid_obj = uuid.UUID(value) + except ValueError: + raise ValidationError( + f"Invalid execution_id '{value}'. Must be a valid UUID." + ) + + # Check if UUID exists in the database + exists = WorkflowExecution.objects.filter(id=uuid_obj).exists() + if not exists: + raise ExecutionDoesNotExistError( + f"Execution with ID '{value}' does not exist." + ) + + return str(uuid_obj) + + class APIDeploymentListSerializer(ModelSerializer): workflow_name = CharField(source="workflow.workflow_name", read_only=True) diff --git a/backend/workflow_manager/workflow_v2/exceptions.py b/backend/workflow_manager/workflow_v2/exceptions.py index 39087ab8e..fa53b3023 100644 --- a/backend/workflow_manager/workflow_v2/exceptions.py +++ b/backend/workflow_manager/workflow_v2/exceptions.py @@ -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" diff --git a/backend/workflow_manager/workflow_v2/workflow_helper.py b/backend/workflow_manager/workflow_v2/workflow_helper.py index afb7962db..c2ecda704 100644 --- a/backend/workflow_manager/workflow_v2/workflow_helper.py +++ b/backend/workflow_manager/workflow_v2/workflow_helper.py @@ -414,17 +414,18 @@ 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() + raise TaskDoesNotExistError( + f"No task ID found for execution: {execution_id}" + ) result = AsyncResult(str(execution.task_id)) - task = AsyncResultData(async_result=result) # Prepare the initial response with the task's current status and result.