From 2b17c60e6042233fe4960fc5e4fcbaca792e55fd Mon Sep 17 00:00:00 2001 From: Marco Riva Date: Tue, 17 Dec 2024 17:35:51 +0100 Subject: [PATCH 1/4] fix error on csv --- api/app/main.py | 4 +++- api/app/models/exceptions.py | 9 +++++++++ api/app/services/file_service.py | 2 +- api/tests/commons/csv_file_mock.py | 4 ++++ api/tests/routes/infer_schema_test.py | 9 ++++++++- api/tests/services/file_service_test.py | 5 +++++ 6 files changed, 30 insertions(+), 3 deletions(-) diff --git a/api/app/main.py b/api/app/main.py index 130ad772..bd83cf78 100644 --- a/api/app/main.py +++ b/api/app/main.py @@ -22,6 +22,7 @@ MetricsError, ModelError, SchemaException, + internal_exception_handler, metrics_exception_handler, model_exception_handler, request_validation_exception_handler, @@ -112,7 +113,7 @@ async def lifespan(fastapi: FastAPI): logger.info('Stopping service ...') -app = FastAPI(title='Radicalbit Platform', lifespan=lifespan) +app = FastAPI(title='Radicalbit Platform', lifespan=lifespan, debug=False) app.add_middleware( CORSMiddleware, @@ -136,3 +137,4 @@ async def lifespan(fastapi: FastAPI): app.add_exception_handler(MetricsError, metrics_exception_handler) app.add_exception_handler(SchemaException, schema_exception_handler) app.add_exception_handler(RequestValidationError, request_validation_exception_handler) +app.add_exception_handler(Exception, internal_exception_handler) diff --git a/api/app/models/exceptions.py b/api/app/models/exceptions.py index 81604ca4..e06abdeb 100644 --- a/api/app/models/exceptions.py +++ b/api/app/models/exceptions.py @@ -4,6 +4,7 @@ from fastapi.encoders import jsonable_encoder from fastapi.exceptions import RequestValidationError from fastapi.responses import JSONResponse +from starlette.requests import Request from app.core import get_config @@ -136,3 +137,11 @@ def request_validation_exception_handler(_, err: RequestValidationError): status_code=422, # https://www.rfc-editor.org/rfc/rfc9110.html#name-422-unprocessable-content content=jsonable_encoder(ErrorOut(error_message)), ) + + +def internal_exception_handler(request: Request, exc: Exception) -> JSONResponse: + logger.error('Internal error occurred [%s]', exc) + return JSONResponse( + status_code=500, + content=jsonable_encoder(ErrorOut(f'Internal server error occurred {exc}')), + ) diff --git a/api/app/services/file_service.py b/api/app/services/file_service.py index 601db2a7..e7102574 100644 --- a/api/app/services/file_service.py +++ b/api/app/services/file_service.py @@ -535,7 +535,7 @@ def validate_file( ) -> None: file_upload_config = get_config().file_upload_config _f_name = csv_file.filename - if csv_file.filename is None: + if csv_file.filename is None or csv_file.size == 0: raise InvalidFileException('The file is empty. Please enter a valid file.') if csv_file.size is not None and csv_file.size >= file_upload_config.max_bytes: raise FileTooLargeException( diff --git a/api/tests/commons/csv_file_mock.py b/api/tests/commons/csv_file_mock.py index 4bdce7e1..cb82e66f 100644 --- a/api/tests/commons/csv_file_mock.py +++ b/api/tests/commons/csv_file_mock.py @@ -90,6 +90,10 @@ def get_uncorrect_sample_csv_file() -> UploadFile: return UploadFile(BytesIO(df.to_csv(index=False).encode()), filename=None) +def get_empty_sample_csv_file() -> UploadFile: + return UploadFile(BytesIO(), filename='sample.csv', size=0) + + def get_file_using_sep(sep: str) -> UploadFile: df = get_dataframe_with_sep(sep) return UploadFile( diff --git a/api/tests/routes/infer_schema_test.py b/api/tests/routes/infer_schema_test.py index d4474fbc..d7de5be8 100644 --- a/api/tests/routes/infer_schema_test.py +++ b/api/tests/routes/infer_schema_test.py @@ -8,7 +8,7 @@ class TestInferSchemaRoute(unittest.TestCase): def setUp(self): - self.client = TestClient(main.app) + self.client = TestClient(main.app, raise_server_exceptions=False) def test_infer_schema(self): file = csv.get_correct_sample_csv_file() @@ -28,3 +28,10 @@ def test_infer_schema_invalid_file(self): def test_infer_schema_no_file(self): response = self.client.post('/api/schema/infer-schema') assert response.status_code == 422 + + def test_infer_schema_empty_file(self): + file = csv.get_empty_sample_csv_file() + response = self.client.post( + '/api/schema/infer-schema', files={'data_file': ('', file.file)} + ) + assert response.status_code == 422 diff --git a/api/tests/services/file_service_test.py b/api/tests/services/file_service_test.py index 2cdddcd6..ed57b89f 100644 --- a/api/tests/services/file_service_test.py +++ b/api/tests/services/file_service_test.py @@ -64,6 +64,11 @@ def test_validate_file_error(self): with pytest.raises(InvalidFileException): self.files_service.validate_file(file, sep=',', columns=['a', 'b']) + def test_validate_empty_file_error(self): + file = csv.get_empty_sample_csv_file() + with pytest.raises(InvalidFileException): + self.files_service.validate_file(file, sep=',', columns=[]) + def test_infer_schema_ok(self): file = csv.get_correct_sample_csv_file() schema = FileService.infer_schema(file) From b53bdeb3e2eaa152413eb933d62c6e27a4ecb7ba Mon Sep 17 00:00:00 2001 From: Marco Riva Date: Tue, 17 Dec 2024 17:45:26 +0100 Subject: [PATCH 2/4] refactor: improve test --- api/app/main.py | 2 +- api/tests/routes/healthcheck_test.py | 2 +- api/tests/routes/metrics_route_test.py | 2 +- api/tests/routes/model_route_test.py | 2 +- api/tests/routes/modelin_validation_test.py | 2 +- api/tests/routes/upload_dataset_route_test.py | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/app/main.py b/api/app/main.py index bd83cf78..84941e9b 100644 --- a/api/app/main.py +++ b/api/app/main.py @@ -113,7 +113,7 @@ async def lifespan(fastapi: FastAPI): logger.info('Stopping service ...') -app = FastAPI(title='Radicalbit Platform', lifespan=lifespan, debug=False) +app = FastAPI(title='Radicalbit Platform', lifespan=lifespan) app.add_middleware( CORSMiddleware, diff --git a/api/tests/routes/healthcheck_test.py b/api/tests/routes/healthcheck_test.py index cb4825a6..46eb1238 100644 --- a/api/tests/routes/healthcheck_test.py +++ b/api/tests/routes/healthcheck_test.py @@ -2,7 +2,7 @@ from app import main -client = TestClient(main.app) +client = TestClient(main.app, raise_server_exceptions=False) def test_healthcheck(): diff --git a/api/tests/routes/metrics_route_test.py b/api/tests/routes/metrics_route_test.py index a35cb822..8208a62a 100644 --- a/api/tests/routes/metrics_route_test.py +++ b/api/tests/routes/metrics_route_test.py @@ -37,7 +37,7 @@ def setUpClass(cls): app.add_exception_handler(MetricsError, metrics_exception_handler) app.include_router(router, prefix=cls.prefix) - cls.client = TestClient(app) + cls.client = TestClient(app, raise_server_exceptions=False) def test_get_reference_statistics_by_model_by_uuid(self): model_uuid = uuid.uuid4() diff --git a/api/tests/routes/model_route_test.py b/api/tests/routes/model_route_test.py index b7288bb0..6b68eb1a 100644 --- a/api/tests/routes/model_route_test.py +++ b/api/tests/routes/model_route_test.py @@ -35,7 +35,7 @@ def setUpClass(cls): app.add_exception_handler(ModelError, model_exception_handler) app.include_router(router, prefix=cls.prefix) - cls.client = TestClient(app) + cls.client = TestClient(app, raise_server_exceptions=False) def test_create_model(self): model_in = db_mock.get_sample_model_in() diff --git a/api/tests/routes/modelin_validation_test.py b/api/tests/routes/modelin_validation_test.py index 5baa701e..56a22d56 100644 --- a/api/tests/routes/modelin_validation_test.py +++ b/api/tests/routes/modelin_validation_test.py @@ -15,7 +15,7 @@ class TestValidationModelRoute(unittest.TestCase): def setUp(self): self.model_service = MagicMock(spec_set=ModelService) self.prefix = '/api/models' - self.client = TestClient(main.app) + self.client = TestClient(main.app, raise_server_exceptions=False) def test_request_validation_exception_handler(self): modelin_data = get_model_sample_wrong( diff --git a/api/tests/routes/upload_dataset_route_test.py b/api/tests/routes/upload_dataset_route_test.py index 27b21cab..74316e85 100644 --- a/api/tests/routes/upload_dataset_route_test.py +++ b/api/tests/routes/upload_dataset_route_test.py @@ -32,7 +32,7 @@ def setUpClass(cls): app = FastAPI(title='Radicalbit Platform', debug=True) app.include_router(router, prefix=cls.prefix) - cls.client = TestClient(app) + cls.client = TestClient(app, raise_server_exceptions=False) def test_upload_reference(self): file = csv.get_correct_sample_csv_file() From 5d84e6535c6e0430c94e32ebd8b1933bcb1de459 Mon Sep 17 00:00:00 2001 From: Marco Riva Date: Tue, 17 Dec 2024 17:58:58 +0100 Subject: [PATCH 3/4] refactor: unused variable --- api/app/models/exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/app/models/exceptions.py b/api/app/models/exceptions.py index e06abdeb..7abd6033 100644 --- a/api/app/models/exceptions.py +++ b/api/app/models/exceptions.py @@ -139,7 +139,7 @@ def request_validation_exception_handler(_, err: RequestValidationError): ) -def internal_exception_handler(request: Request, exc: Exception) -> JSONResponse: +def internal_exception_handler(_, exc: Exception) -> JSONResponse: logger.error('Internal error occurred [%s]', exc) return JSONResponse( status_code=500, From 6571636c65ec33d73842456f5cc122c334200b58 Mon Sep 17 00:00:00 2001 From: Marco Riva Date: Tue, 17 Dec 2024 18:01:04 +0100 Subject: [PATCH 4/4] refactor: unused variable --- api/app/models/exceptions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/app/models/exceptions.py b/api/app/models/exceptions.py index 7abd6033..7d8384d2 100644 --- a/api/app/models/exceptions.py +++ b/api/app/models/exceptions.py @@ -4,7 +4,6 @@ from fastapi.encoders import jsonable_encoder from fastapi.exceptions import RequestValidationError from fastapi.responses import JSONResponse -from starlette.requests import Request from app.core import get_config