many fixes

Author: sanderegg

packages/pytest-simcore/src/pytest_simcore/helpers/httpx_assert_checks.py

@@ -44,7 +44,11 @@ def assert_status(
         error = validated_response.error
         if is_error(expected_status_code):
             _do_assert_error(
-                data, error, expected_status_code, expected_msg, expected_error_code
+                data,
+                error,
+                expected_status_code,
+                expected_msg,
+                expected_error_code,
             )
         return data, error
 
@@ -60,8 +64,8 @@ def _do_assert_error(
     data,
     error,
     expected_status_code: int,
-    expected_msg: str | None = None,
-    expected_error_code: str | None = None,
+    expected_msg: str | None,
+    expected_error_code: str | None,
 ):
     assert not data, pformat(data)
     assert error, pformat(error)

packages/pytest-simcore/src/pytest_simcore/helpers/s3.py

@@ -4,12 +4,12 @@
 from typing import Final
 
 import aiofiles
+import httpx
 import orjson
-from aiohttp import ClientSession
 from aws_library.s3 import MultiPartUploadLinks
+from fastapi import status
 from models_library.api_schemas_storage import ETag, FileUploadSchema, UploadedPart
 from pydantic import AnyUrl, ByteSize, TypeAdapter
-from servicelib.aiohttp import status
 from servicelib.utils import limited_as_completed, logged_gather
 from types_aiobotocore_s3 import S3Client
 
@@ -37,7 +37,7 @@ async def _file_sender(
 
 
 async def upload_file_part(
-    session: ClientSession,
+    session: httpx.AsyncClient,
     file: Path,
     part_index: int,
     file_offset: int,
@@ -48,11 +48,11 @@ async def upload_file_part(
     raise_while_uploading: bool = False,
 ) -> tuple[int, ETag]:
     print(
-        f"--> uploading {this_file_chunk_size=} of {file=}, [{part_index+1}/{num_parts}]..."
+        f"--> uploading {this_file_chunk_size=} of {file=}, [{part_index + 1}/{num_parts}]..."
     )
     response = await session.put(
         str(upload_url),
-        data=_file_sender(
+        content=_file_sender(
             file,
             offset=file_offset,
             bytes_to_send=this_file_chunk_size,
@@ -64,12 +64,12 @@ async def upload_file_part(
     )
     response.raise_for_status()
     # NOTE: the response from minio does not contain a json body
-    assert response.status == status.HTTP_200_OK
+    assert response.status_code == status.HTTP_200_OK
     assert response.headers
     assert "Etag" in response.headers
     received_e_tag = orjson.loads(response.headers["Etag"])
     print(
-        f"--> completed upload {this_file_chunk_size=} of {file=}, [{part_index+1}/{num_parts}], {received_e_tag=}"
+        f"--> completed upload {this_file_chunk_size=} of {file=}, [{part_index + 1}/{num_parts}], {received_e_tag=}"
     )
     return (part_index, received_e_tag)
 
@@ -80,7 +80,7 @@ async def upload_file_to_presigned_link(
     file_size = file.stat().st_size
 
     with log_context(logging.INFO, msg=f"uploading {file} via {file_upload_link=}"):
-        async with ClientSession() as session:
+        async with httpx.AsyncClient() as session:
             file_chunk_size = int(file_upload_link.chunk_size)
             num_urls = len(file_upload_link.urls)
             last_chunk_size = file_size - file_chunk_size * (num_urls - 1)
@@ -100,7 +100,7 @@ async def upload_file_to_presigned_link(
                         upload_url,
                     )
                 )
-            results = await logged_gather(*upload_tasks, max_concurrency=0)
+            results = await logged_gather(*upload_tasks, max_concurrency=20)
         return [UploadedPart(number=index + 1, e_tag=e_tag) for index, e_tag in results]
 
 

packages/service-library/src/servicelib/fastapi/http_error.py

@@ -1,34 +1,28 @@
 from collections.abc import Awaitable, Callable
 
-from fastapi import HTTPException, status
+from fastapi import HTTPException
 from fastapi.encoders import jsonable_encoder
-from fastapi.exceptions import RequestValidationError
 from fastapi.openapi.constants import REF_PREFIX
 from fastapi.openapi.utils import validation_error_response_definition
 from fastapi.requests import Request
 from fastapi.responses import JSONResponse
-from pydantic import ValidationError
 
 
-async def http_error_handler(_: Request, exc: Exception) -> JSONResponse:
-    assert isinstance(exc, HTTPException)
-
-    return JSONResponse(
-        content=jsonable_encoder({"errors": [exc.detail]}), status_code=exc.status_code
-    )
-
+def make_default_http_error_handler(
+    *, envelope_error: bool
+) -> Callable[[Request, Exception], Awaitable[JSONResponse]]:
+    async def _http_error_handler(_: Request, exc: Exception) -> JSONResponse:
+        assert isinstance(exc, HTTPException)
 
-async def http422_error_handler(
-    request: Request,
-    exc: Exception,
-) -> JSONResponse:
-    assert request  # nosec
-    assert isinstance(exc, RequestValidationError | ValidationError)
+        error_content = {"errors": [exc.detail]}
+        if envelope_error:
+            error_content = {"error": error_content}
+        return JSONResponse(
+            content=jsonable_encoder(error_content),
+            status_code=exc.status_code,
+        )
 
-    return JSONResponse(
-        content=jsonable_encoder({"errors": exc.errors()}),
-        status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
-    )
+    return _http_error_handler
 
 
 validation_error_response_definition["properties"] = {
@@ -41,7 +35,10 @@ async def http422_error_handler(
 
 
 def make_http_error_handler_for_exception(
-    status_code: int, exception_cls: type[BaseException]
+    status_code: int,
+    exception_cls: type[BaseException],
+    *,
+    envelope_error: bool,
 ) -> Callable[[Request, Exception], Awaitable[JSONResponse]]:
     """
     Produces a handler for BaseException-type exceptions which converts them
@@ -52,8 +49,11 @@ def make_http_error_handler_for_exception(
 
     async def _http_error_handler(_: Request, exc: Exception) -> JSONResponse:
         assert isinstance(exc, exception_cls)  # nosec
+        error_content = {"errors": [f"{exc}"]}
+        if envelope_error:
+            error_content = {"error": error_content}
         return JSONResponse(
-            content=jsonable_encoder({"errors": [str(exc)]}),
+            content=jsonable_encoder(error_content),
             status_code=status_code,
         )
 

services/storage/src/simcore_service_storage/api/rest/_files.py

@@ -1,6 +1,7 @@
 import asyncio
 import logging
 import socket
+import urllib.parse
 from typing import Annotated, cast
 
 from fastapi import APIRouter, Depends, Header, HTTPException, Request
@@ -301,7 +302,9 @@ async def is_completed_upload_file(
     # if it returns slow we return a 202 - Accepted, the client will have to check later
     # for completeness
     task_name = create_upload_completion_task_name(query_params.user_id, file_id)
-    assert task_name == future_id  # nosec
+    assert task_name == urllib.parse.quote(
+        future_id
+    )  # nosec # NOTE: fastapi auto-decode path parameters
     # first check if the task is in the app
     if task := get_completed_upload_tasks(request.app).get(task_name):
         if task.done():

services/storage/src/simcore_service_storage/exceptions/handlers.py

@@ -4,9 +4,9 @@
 from aws_library.s3 import S3AccessError, S3KeyNotFoundError
 from fastapi import FastAPI, HTTPException, status
 from fastapi.exceptions import RequestValidationError
+from pydantic import ValidationError
 from servicelib.fastapi.http_error import (
-    http422_error_handler,
-    http_error_handler,
+    make_default_http_error_handler,
     make_http_error_handler_for_exception,
 )
 
@@ -26,13 +26,34 @@
 
 
 def set_exception_handlers(app: FastAPI) -> None:
-    app.add_exception_handler(HTTPException, http_error_handler)
-    app.add_exception_handler(RequestValidationError, http422_error_handler)
-    # director-v2 core.errors mappend into HTTP errors
+    app.add_exception_handler(
+        HTTPException, make_default_http_error_handler(envelope_error=True)
+    )
+    app.add_exception_handler(
+        HTTPException,
+        make_http_error_handler_for_exception(
+            status.HTTP_422_UNPROCESSABLE_ENTITY,
+            HTTPException,
+            envelope_error=True,
+        ),
+    )
+
+    for exc_unprocessable in (ValidationError, RequestValidationError):
+        app.add_exception_handler(
+            exc_unprocessable,
+            make_http_error_handler_for_exception(
+                status.HTTP_422_UNPROCESSABLE_ENTITY,
+                exc_unprocessable,
+                envelope_error=True,
+            ),
+        )
+
     app.add_exception_handler(
         InvalidFileIdentifierError,
         make_http_error_handler_for_exception(
-            status.HTTP_422_UNPROCESSABLE_ENTITY, InvalidFileIdentifierError
+            status.HTTP_422_UNPROCESSABLE_ENTITY,
+            InvalidFileIdentifierError,
+            envelope_error=True,
         ),
     )
     for exc_not_found in (
@@ -43,7 +64,7 @@ def set_exception_handlers(app: FastAPI) -> None:
         app.add_exception_handler(
             exc_not_found,
             make_http_error_handler_for_exception(
-                status.HTTP_404_NOT_FOUND, exc_not_found
+                status.HTTP_404_NOT_FOUND, exc_not_found, envelope_error=True
             ),
         )
     for exc_access in (
@@ -53,13 +74,15 @@ def set_exception_handlers(app: FastAPI) -> None:
         app.add_exception_handler(
             exc_access,
             make_http_error_handler_for_exception(
-                status.HTTP_403_FORBIDDEN, exc_access
+                status.HTTP_403_FORBIDDEN, exc_access, envelope_error=True
             ),
         )
     app.add_exception_handler(
         LinkAlreadyExistsError,
         make_http_error_handler_for_exception(
-            status.HTTP_422_UNPROCESSABLE_ENTITY, LinkAlreadyExistsError
+            status.HTTP_422_UNPROCESSABLE_ENTITY,
+            LinkAlreadyExistsError,
+            envelope_error=True,
         ),
     )
     for exc_3rd_party in (
@@ -69,27 +92,29 @@ def set_exception_handlers(app: FastAPI) -> None:
         app.add_exception_handler(
             exc_3rd_party,
             make_http_error_handler_for_exception(
-                status.HTTP_503_SERVICE_UNAVAILABLE, exc_3rd_party
+                status.HTTP_503_SERVICE_UNAVAILABLE, exc_3rd_party, envelope_error=True
             ),
         )
 
     app.add_exception_handler(
         DatcoreAdapterTimeoutError,
         make_http_error_handler_for_exception(
-            status.HTTP_504_GATEWAY_TIMEOUT, DatcoreAdapterTimeoutError
+            status.HTTP_504_GATEWAY_TIMEOUT,
+            DatcoreAdapterTimeoutError,
+            envelope_error=True,
         ),
     )
 
     # SEE https://docs.python.org/3/library/exceptions.html#exception-hierarchy
     app.add_exception_handler(
         NotImplementedError,
         make_http_error_handler_for_exception(
-            status.HTTP_501_NOT_IMPLEMENTED, NotImplementedError
+            status.HTTP_501_NOT_IMPLEMENTED, NotImplementedError, envelope_error=True
         ),
     )
     app.add_exception_handler(
         Exception,
         make_http_error_handler_for_exception(
-            status.HTTP_500_INTERNAL_SERVER_ERROR, Exception
+            status.HTTP_500_INTERNAL_SERVER_ERROR, Exception, envelope_error=True
         ),
     )

services/storage/src/simcore_service_storage/models.py

@@ -186,13 +186,6 @@ class SyncMetadataResponse(BaseModel):
 class FileDownloadQueryParams(StorageQueryParamsBase):
     link_type: LinkType = LinkType.PRESIGNED
 
-    @field_validator("link_type", mode="before")
-    @classmethod
-    def convert_from_lower_case(cls, v: str) -> str:
-        if v is not None:
-            return f"{v}".upper()
-        return v
-
 
 class FileDownloadResponse(BaseModel):
     link: AnyUrl

services/storage/src/simcore_service_storage/modules/db/file_meta_data.py

@@ -40,7 +40,6 @@ async def upsert(
         index_elements=[file_meta_data.c.file_id], set_=fmd_db.model_dump()
     ).returning(literal_column("*"))
     result = await conn.execute(on_update_statement)
-    await conn.commit()
     row = result.one()
     return FileMetaDataAtDB.model_validate(row)
 

services/storage/src/simcore_service_storage/simcore_s3_dsm.py

@@ -445,15 +445,15 @@ async def create_file_download_link(
             directory_file_id: SimcoreS3FileID | None = await get_directory_file_id(
                 conn, cast(SimcoreS3FileID, file_id)
             )
-            await self.__ensure_read_access_rights(
+            await self._ensure_read_access_rights(
                 conn, user_id, directory_file_id if directory_file_id else file_id
             )
         if directory_file_id:
             if not await get_s3_client(self.app).object_exists(
                 bucket=self.simcore_bucket_name, object_key=f"{file_id}"
             ):
                 raise S3KeyNotFoundError(key=file_id, bucket=self.simcore_bucket_name)
-            return await self.__get_link(
+            return await self._get_link(
                 TypeAdapter(SimcoreS3FileID).validate_python(file_id), link_type
             )
         # standard file link
@@ -464,10 +464,10 @@ async def create_file_download_link(
         if not is_file_entry_valid(fmd):
             # try lazy update
             fmd = await self._update_database_from_storage(fmd)
-        return await self.__get_link(fmd.object_name, link_type)
+        return await self._get_link(fmd.object_name, link_type)
 
     @staticmethod
-    async def __ensure_read_access_rights(
+    async def _ensure_read_access_rights(
         conn: AsyncConnection, user_id: UserID, storage_file_id: StorageFileID
     ) -> None:
         can = await get_file_access_rights(conn, user_id, storage_file_id)
@@ -478,7 +478,7 @@ async def __ensure_read_access_rights(
             #
             raise FileAccessRightError(access_right="read", file_id=storage_file_id)
 
-    async def __get_link(
+    async def _get_link(
         self, s3_file_id: SimcoreS3FileID, link_type: LinkType
     ) -> AnyUrl:
         link: AnyUrl = TypeAdapter(AnyUrl).validate_python(

services/storage/tests/conftest.py

@@ -473,8 +473,8 @@ async def _directory_creator(dir_name: str):
         # 3. call file_upload_complete_response until it replies OK
 
         directory_file_id = create_simcore_file_id(project_id, node_id, dir_name)
-        directory_file_upload: FileUploadSchema = await create_upload_file_link_v2(
-            directory_file_id, link_type="s3", is_directory="true", file_size=-1
+        directory_file_upload = await create_upload_file_link_v2(
+            directory_file_id, link_type="S3", is_directory="true", file_size=-1
         )
         # always returns a v2 link when dealing with directories
         assert isinstance(directory_file_upload, FileUploadSchema)