From 82f2f32af76ff8693ef2b14d9017fabf0940b033 Mon Sep 17 00:00:00 2001 From: Mehmed Mustafa Date: Fri, 1 Nov 2024 15:32:56 +0100 Subject: [PATCH] refine: oton endpoint --- .../operandi_server/routers/workflow.py | 36 +++++-------------- .../operandi_server/routers/workflow_utils.py | 31 ++++++++++++++-- src/server/operandi_server/server.py | 4 ++- .../operandi_utils/oton/ocrd_validator.py | 2 +- .../operandi_utils/oton/oton_converter.py | 1 - tests/tests_server/test_endpoint_workflow.py | 2 +- ...2_validator_2_validate_processor_params.py | 8 ++--- 7 files changed, 46 insertions(+), 38 deletions(-) diff --git a/src/server/operandi_server/routers/workflow.py b/src/server/operandi_server/routers/workflow.py index 01363806..c5194a50 100644 --- a/src/server/operandi_server/routers/workflow.py +++ b/src/server/operandi_server/routers/workflow.py @@ -17,7 +17,6 @@ from operandi_utils.database import ( db_create_workflow, db_create_workflow_job, db_get_hpc_slurm_job, db_get_workflow, db_update_workspace, db_increase_processing_stats_with_handling) -from operandi_utils.oton import OTONConverter from operandi_utils.rabbitmq import ( get_connection_publisher, RABBITMQ_QUEUE_JOB_STATUSES, RABBITMQ_QUEUE_HARVESTER, RABBITMQ_QUEUE_USERS) from operandi_server.constants import ( @@ -30,7 +29,8 @@ convert_oton_with_handling, get_db_workflow_job_with_handling, get_db_workflow_with_handling, - nf_script_uses_mets_server_with_handling + nf_script_uses_mets_server_with_handling, + validate_oton_with_handling ) from .workspace_utils import check_if_file_group_exists_with_handling, get_db_workspace_with_handling from .user import RouterUser @@ -51,7 +51,7 @@ def __init__(self): self.router = APIRouter(tags=[ServerApiTag.WORKFLOW]) self.router.add_api_route( path=f"/workflow", endpoint=self.list_workflows, methods=["GET"], status_code=status.HTTP_200_OK, - summary="Get a list of existing nextflow workflows.", + summary="Get a list of existing nextflow from operandi_utils.oton import OTONConverter, OCRDValidatorworkflows.", response_model=List[WorkflowRsrc], response_model_exclude_unset=True, response_model_exclude_none=True ) self.router.add_api_route( @@ -108,13 +108,12 @@ def __init__(self): # Added by Faizan self.router.add_api_route( path="/convert_workflow", - endpoint=self.convert_txt_to_nextflow, - methods=["POST"], - status_code=status.HTTP_201_CREATED, + endpoint=self.convert_txt_to_nextflow, methods=["POST"], status_code=status.HTTP_201_CREATED, summary=""" Upload a text file containing a workflow in ocrd process format and convert it to a Nextflow script in the desired format (local/docker) - """ + """, + response_model=None, response_model_exclude_unset=False, response_model_exclude_none=False ) def __del__(self): @@ -448,12 +447,6 @@ async def convert_txt_to_nextflow( # Authenticate the user await self.user_authenticator.user_login(auth) - environments = ["local", "docker", "apptainer"] - if environment not in environments: - message = f"Unknown environment value: {environment}. Must be one of: {environments}" - self.logger.error(message) - raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=message) - oton_id, oton_dir = create_resource_dir(SERVER_OTON_CONVERSIONS, resource_id=None) ocrd_process_txt = join(oton_dir, f"ocrd_process_input.txt") nf_script_dest = join(oton_dir, f"nextflow_output.nf") @@ -465,19 +458,6 @@ async def convert_txt_to_nextflow( self.logger.error(f"{message}, error: {error}") raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=message) - # Use the Converter's convert_OtoN function instead of directly calling OCRDValidator - converter = OTONConverter() - try: - # Call the conversion function (this will also perform validation inside) - if environment == "local": - converter.convert_oton_env_local(str(ocrd_process_txt), str(nf_script_dest)) - elif environment == "docker": - converter.convert_oton_env_docker(str(ocrd_process_txt), str(nf_script_dest)) - elif environment == "apptainer": - converter.convert_oton_env_apptainer(str(ocrd_process_txt), str(nf_script_dest)) - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) - - # Return the generated Nextflow (.nf) file as a response - + await validate_oton_with_handling(self.logger, ocrd_process_txt) + await convert_oton_with_handling(self.logger, environment, ocrd_process_txt, nf_script_dest) return FileResponse(nf_script_dest, filename=f'{oton_id}.nf') diff --git a/src/server/operandi_server/routers/workflow_utils.py b/src/server/operandi_server/routers/workflow_utils.py index e5b49dfc..7401881b 100644 --- a/src/server/operandi_server/routers/workflow_utils.py +++ b/src/server/operandi_server/routers/workflow_utils.py @@ -3,6 +3,7 @@ from operandi_utils.database import db_get_workflow, db_get_workflow_job from operandi_utils.database.models import DBWorkflow, DBWorkflowJob +from operandi_utils.oton import OTONConverter, OCRDValidator async def get_db_workflow_with_handling( @@ -56,5 +57,31 @@ async def nf_script_uses_mets_server_with_handling( raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=message) -async def convert_oton_with_handling(): - pass +async def validate_oton_with_handling(logger, ocrd_process_txt_path: str): + try: + # Separate validation for refined error logging + validator = OCRDValidator() + validator.validate(input_file=ocrd_process_txt_path) + except ValueError as error: + message = "Failed to validate the ocrd process workflow txt file" + logger.error(f"{message}, error: {error}") + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=message) + +async def convert_oton_with_handling(logger, environment: str, ocrd_process_txt_path: str, nf_script_dest_path: str): + environments = ["local", "docker", "apptainer"] + if environment not in environments: + message = f"Unknown environment value: {environment}. Must be one of: {environments}" + logger.error(message) + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=message) + try: + converter = OTONConverter() + if environment == "local": + converter.convert_oton_env_local(str(ocrd_process_txt_path), str(nf_script_dest_path)) + elif environment == "docker": + converter.convert_oton_env_docker(str(ocrd_process_txt_path), str(nf_script_dest_path)) + elif environment == "apptainer": + converter.convert_oton_env_apptainer(str(ocrd_process_txt_path), str(nf_script_dest_path)) + except ValueError as error: + message = "Failed to convert ocrd process workflow to nextflow workflow" + logger.error(f"{message}, error: {error}") + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=message) diff --git a/src/server/operandi_server/server.py b/src/server/operandi_server/server.py index 2e86bd83..c413cf2f 100644 --- a/src/server/operandi_server/server.py +++ b/src/server/operandi_server/server.py @@ -10,7 +10,8 @@ from operandi_utils.database import db_initiate_database from operandi_utils import safe_init_logging -from operandi_server.constants import SERVER_WORKFLOW_JOBS_ROUTER, SERVER_WORKFLOWS_ROUTER, SERVER_WORKSPACES_ROUTER +from operandi_server.constants import ( + SERVER_OTON_CONVERSIONS, SERVER_WORKFLOW_JOBS_ROUTER, SERVER_WORKFLOWS_ROUTER, SERVER_WORKSPACES_ROUTER) from operandi_server.files_manager import create_resource_base_dir from operandi_server.routers import RouterAdminPanel, RouterDiscovery, RouterUser, RouterWorkflow, RouterWorkspace from operandi_server.routers.user_utils import create_user_if_not_available @@ -81,6 +82,7 @@ async def startup_event(self): # Reconfigure all loggers to the same format reconfigure_all_loggers(log_level=LOG_LEVEL_SERVER, log_file_path=log_file_path) + create_resource_base_dir(SERVER_OTON_CONVERSIONS) create_resource_base_dir(SERVER_WORKFLOW_JOBS_ROUTER) create_resource_base_dir(SERVER_WORKFLOWS_ROUTER) create_resource_base_dir(SERVER_WORKSPACES_ROUTER) diff --git a/src/utils/operandi_utils/oton/ocrd_validator.py b/src/utils/operandi_utils/oton/ocrd_validator.py index b79d7c0a..f50153e9 100644 --- a/src/utils/operandi_utils/oton/ocrd_validator.py +++ b/src/utils/operandi_utils/oton/ocrd_validator.py @@ -64,7 +64,7 @@ def validate_processor_params( report = ParameterValidator(processor_args.ocrd_tool_json).validate(processor_args.parameters) if not report.is_valid: self.logger.error(report.errors) - raise Exception(report.errors) + raise ValueError(report.errors) # Remove the overwritten defaults to keep the produced NF executable file less populated # Note: The defaults, still get overwritten in run-time diff --git a/src/utils/operandi_utils/oton/oton_converter.py b/src/utils/operandi_utils/oton/oton_converter.py index e423b8c8..65ff07ff 100755 --- a/src/utils/operandi_utils/oton/oton_converter.py +++ b/src/utils/operandi_utils/oton/oton_converter.py @@ -7,7 +7,6 @@ def __init__(self): self.ocrd_validator = OCRDValidator() def __convert_oton(self, input_path: str, output_path: str, environment: str): - self.ocrd_validator.validate(input_path) list_processor_call_arguments = self.ocrd_validator.validate(input_path) nf_file_executable = NextflowFileExecutable() if environment == "local": diff --git a/tests/tests_server/test_endpoint_workflow.py b/tests/tests_server/test_endpoint_workflow.py index e1cef024..fac64915 100644 --- a/tests/tests_server/test_endpoint_workflow.py +++ b/tests/tests_server/test_endpoint_workflow.py @@ -167,7 +167,7 @@ def test_convert_txt_to_nextflow_validator_failure(operandi, auth): response = operandi.post(url="/convert_workflow", files=files, auth=auth, params=params) assert_response_status_code(response.status_code, expected_floor=4) - assert "Invalid first line. Expected: 'ocrd process', got: 'Invalid ocrd process text" in response.json()["detail"] + assert "Failed to validate the ocrd process workflow txt file" in response.json()["detail"] # Added by Faizan diff --git a/tests/tests_utils/test_2_oton/test_2_validator_2_validate_processor_params.py b/tests/tests_utils/test_2_oton/test_2_validator_2_validate_processor_params.py index ba05b423..a7b3052c 100644 --- a/tests/tests_utils/test_2_oton/test_2_validator_2_validate_processor_params.py +++ b/tests/tests_utils/test_2_oton/test_2_validator_2_validate_processor_params.py @@ -46,16 +46,16 @@ def test_falseness_basic(ocrd_parser, ocrd_validator): def test_falseness_with_params_separated(ocrd_parser, ocrd_validator): call_args: ProcessorCallArguments = ocrd_parser.parse_arguments( "cis-ocropy-binarize -I OCR-D-IMG -O OCR-D-BIN -P dpi 177.0 -P non-existing-param 0.42") - with raises(Exception): + with raises(ValueError): ocrd_validator.validate_processor_params(processor_args=call_args, overwrite_with_defaults=False) - with raises(Exception): + with raises(ValueError): ocrd_validator.validate_processor_params(processor_args=call_args, overwrite_with_defaults=True) def test_falseness_with_params_clustered(ocrd_parser, ocrd_validator): call_args: ProcessorCallArguments = ocrd_parser.parse_arguments( """cis-ocropy-binarize -I OCR-D-IMG -O OCR-D-BIN -p '{"dpi": 177.0, "non-existing-param": 0.42}'""") - with raises(Exception): + with raises(ValueError): ocrd_validator.validate_processor_params(processor_args=call_args, overwrite_with_defaults=False) - with raises(Exception): + with raises(ValueError): ocrd_validator.validate_processor_params(processor_args=call_args, overwrite_with_defaults=True)