♻️⬆️ Upgrading parfive (ITISFoundation#2720)

* upgraded parfive version and removed constraing
* better error reporting in UI
* ParallelDownoader reporters errors in the logs

Author: GitHK

services/web/server/requirements/_base.in

@@ -69,7 +69,7 @@ expiringdict
 semantic_version
 
 # import/export excel
-parfive==1.0.2 ## See note  in simcore_service_webserver/exporter/file_downloader.py
+parfive
 openpyxl
 python-magic
 

services/web/server/requirements/_base.txt

@@ -150,7 +150,7 @@ orjson==3.5.3
     # via -r requirements/_base.in
 pamqp==2.3.0
     # via aiormq
-parfive==1.0.2
+parfive==1.5.1
     # via -r requirements/_base.in
 passlib==1.7.4
     # via -r requirements/_base.in

services/web/server/src/simcore_service_webserver/exporter/file_downloader.py

@@ -1,8 +1,6 @@
 import logging
 from pathlib import Path
 
-import parfive
-from aiofiles import os as aiofiles_os
 from aiohttp.web import Application
 from parfive.downloader import Downloader
 
@@ -13,44 +11,32 @@
 log = logging.getLogger(__name__)
 
 
-if parfive.__version__ != "1.0.2":
-    raise RuntimeError(
-        "Parfive was upgraded, please make sure this version supports "
-        "aiofiles otherwise it will block the main loop while downloading files. "
-        "If such condition is not met please do not upgrade! "
-        "A PR to parfive will be submitted by GitHK and this should be no longer required."
-    )
-
-
 class ParallelDownloader:
     def __init__(self):
         self.downloader = Downloader(
             progress=False, file_progress=False, notebook=False, overwrite=True
         )
         self.total_files_added = 0
 
-    async def append_file(self, link: str, download_path: Path):
+    async def append_file(self, link: str, download_path: Path) -> None:
         await makedirs(download_path.parent, exist_ok=True)
         self.downloader.enqueue_file(
             url=link, path=download_path.parent, filename=download_path.name
         )
         self.total_files_added += 1
 
-    async def download_files(self, app: Application):
+    async def download_files(self, app: Application) -> None:
         """starts the download and waits for all files to finish"""
-
-        # run this async, parfive will support aiofiles in the future as stated above
-        wrapped_function = aiofiles_os.wrap(self.downloader.download)
         exporter_settings = get_settings(app)
-        results = await wrapped_function(
+        results = await self.downloader.run_download(
             timeouts={
                 "total": exporter_settings.downloader_max_timeout_seconds,
                 "sock_read": 90,  # default as in parfive code
             }
         )
-        log.debug("Download results %s", results)
 
-        if len(results) != self.total_files_added:
-            raise ExporterException(
-                "Not all files were downloaded. Please check the logs above."
-            )
+        log.debug("Download %s using %s", f"{results=}", f"{self.downloader=}")
+        if len(results) != self.total_files_added or len(results.errors) > 0:
+            message = f"Not all files were downloaded: {results.errors=}"
+            log.error(message)
+            raise ExporterException(message)

services/web/server/tests/integration/01/test_exporter.py

@@ -31,6 +31,7 @@
 import aiopg.sa
 import aioredis
 import pytest
+from _pytest.monkeypatch import MonkeyPatch
 from aiohttp.test_utils import TestClient
 from models_library.settings.redis import RedisConfig
 from pytest_simcore.docker_registry import _pull_push_service
@@ -607,3 +608,57 @@ async def test_import_export_import_duplicate(
         duplicated_files_checksums,
         condition_operator=operator.eq,
     )
+
+
+@pytest.fixture
+def mock_file_downloader(monkeypatch: MonkeyPatch) -> None:
+    original_append_file = ParallelDownloader.append_file
+
+    async def mock_append_file(self, link: str, download_path: Path) -> None:
+        # making the download fail
+        link = "http://localhost/missing"
+        await original_append_file(self, link, download_path)
+
+    monkeypatch.setattr(ParallelDownloader, "append_file", mock_append_file)
+
+
+async def test_download_error_reporting(
+    client: TestClient,
+    push_services_to_registry: None,
+    aiopg_engine: aiopg.sa.engine.Engine,
+    redis_client: aioredis.Redis,
+    simcore_services_ready: None,
+    grant_access_rights: None,
+    mock_file_downloader: None,
+):
+    await login_user(client)
+
+    # not testing agains all versions, results will be the same
+    export_version = get_exported_projects()[0]
+    export_file_name = export_version.name
+    version_from_name = export_file_name.split("#")[0]
+
+    assert_error = (
+        f"The '{version_from_name}' version' is not present in the supported versions: "
+        f"{SUPPORTED_EXPORTER_VERSIONS}. If it's a new version please remember to add it."
+    )
+    assert version_from_name in SUPPORTED_EXPORTER_VERSIONS, assert_error
+
+    imported_project_uuid = await import_study_from_file(client, export_version)
+
+    headers = {X_PRODUCT_NAME_HEADER: "osparc"}
+
+    # export newly imported project
+    url_export = client.app.router["export_project"].url_for(
+        project_id=imported_project_uuid
+    )
+
+    assert url_export == URL(API_PREFIX + f"/projects/{imported_project_uuid}:xport")
+    async with await client.post(
+        f"{url_export}", headers=headers, timeout=10
+    ) as export_response:
+        assert export_response.status == 400, await export_response.text()
+        json_response = await export_response.json()
+        assert json_response["error"]["logs"][0]["message"].startswith(
+            "Not all files were downloaded: "
+        )