Merge pull request #1648 from fractal-analytics-platform/1644-log-fil…

…es-for-failed-task-are-not-downloadable-from-fractal-web-after-job-execution-error

Correctly zip structured folders

CHANGELOG.md

@@ -1,5 +1,12 @@
 **Note**: Numbers like (\#1234) point to closed Pull Requests on the fractal-server repository.
 
+# 2.3.2
+
+> **WARNING**: The remove-remote-venv-folder in the SSH task collection is broken (see issue 1633). Do not deploy this version in an SSH-based `fractal-server` instance.
+
+* API:
+    * Fix incorrect zipping of structured job-log folders (\#1648).
+
 # 2.3.1
 
 This release includes a bugfix for task names with special characters.

fractal_server/app/routes/admin/v1.py

@@ -387,9 +387,7 @@ async def download_job_logs(
     # Create and return byte stream for zipped log folder
     PREFIX_ZIP = Path(job.working_dir).name
     zip_filename = f"{PREFIX_ZIP}_archive.zip"
-    byte_stream = _zip_folder_to_byte_stream(
-        folder=job.working_dir, zip_filename=zip_filename
-    )
+    byte_stream = _zip_folder_to_byte_stream(folder=job.working_dir)
     return StreamingResponse(
         iter([byte_stream.getvalue()]),
         media_type="application/x-zip-compressed",

fractal_server/app/routes/admin/v2.py

@@ -274,9 +274,7 @@ async def download_job_logs(
     # Create and return byte stream for zipped log folder
     PREFIX_ZIP = Path(job.working_dir).name
     zip_filename = f"{PREFIX_ZIP}_archive.zip"
-    byte_stream = _zip_folder_to_byte_stream(
-        folder=job.working_dir, zip_filename=zip_filename
-    )
+    byte_stream = _zip_folder_to_byte_stream(folder=job.working_dir)
     return StreamingResponse(
         iter([byte_stream.getvalue()]),
         media_type="application/x-zip-compressed",

fractal_server/app/routes/api/v1/job.py

@@ -128,9 +128,7 @@ async def download_job_logs(
     # Create and return byte stream for zipped log folder
     PREFIX_ZIP = Path(job.working_dir).name
     zip_filename = f"{PREFIX_ZIP}_archive.zip"
-    byte_stream = _zip_folder_to_byte_stream(
-        folder=job.working_dir, zip_filename=zip_filename
-    )
+    byte_stream = _zip_folder_to_byte_stream(folder=job.working_dir)
     return StreamingResponse(
         iter([byte_stream.getvalue()]),
         media_type="application/x-zip-compressed",

fractal_server/app/routes/api/v2/job.py

@@ -131,9 +131,7 @@ async def download_job_logs(
     # Create and return byte stream for zipped log folder
     PREFIX_ZIP = Path(job.working_dir).name
     zip_filename = f"{PREFIX_ZIP}_archive.zip"
-    byte_stream = _zip_folder_to_byte_stream(
-        folder=job.working_dir, zip_filename=zip_filename
-    )
+    byte_stream = _zip_folder_to_byte_stream(folder=job.working_dir)
     return StreamingResponse(
         iter([byte_stream.getvalue()]),
         media_type="application/x-zip-compressed",

fractal_server/app/routes/aux/_job.py

@@ -1,3 +1,4 @@
+import os
 from io import BytesIO
 from pathlib import Path
 from typing import Union
@@ -25,19 +26,20 @@ def _write_shutdown_file(*, job: Union[ApplyWorkflow, JobV2]):
         f.write(f"Trigger executor shutdown for {job.id=}.")
 
 
-def _zip_folder_to_byte_stream(*, folder: str, zip_filename: str) -> BytesIO:
+def _zip_folder_to_byte_stream(*, folder: str) -> BytesIO:
     """
     Get byte stream with the zipped log folder of a job.
 
     Args:
         folder: the folder to zip
-        zip_filename: name of the zipped archive
     """
-    working_dir_path = Path(folder)
 
     byte_stream = BytesIO()
     with ZipFile(byte_stream, mode="w", compression=ZIP_DEFLATED) as zipfile:
-        for fpath in working_dir_path.glob("*"):
-            zipfile.write(filename=str(fpath), arcname=str(fpath.name))
+        for root, dirs, files in os.walk(folder):
+            for file in files:
+                file_path = os.path.join(root, file)
+                archive_path = os.path.relpath(file_path, folder)
+                zipfile.write(file_path, archive_path)
 
     return byte_stream

tests/no_version/test_unit_test_zip_folder_to_byte_stream.py

@@ -0,0 +1,39 @@
+from pathlib import Path
+from zipfile import ZipFile
+
+from devtools import debug
+
+from fractal_server.app.routes.aux._job import _zip_folder_to_byte_stream
+
+
+def test_zip_folder_to_byte_stream(tmp_path: Path):
+    debug(tmp_path)
+
+    # Prepare file/folder structure
+    (tmp_path / "file1").touch()
+    (tmp_path / "file2").touch()
+    (tmp_path / "folder").mkdir()
+    (tmp_path / "folder/file3").touch()
+    (tmp_path / "folder/file4").touch()
+
+    output = _zip_folder_to_byte_stream(folder=tmp_path.as_posix())
+
+    # Write BytesIO to file
+    archive_path = tmp_path / "zipped_folder.zip"
+    with archive_path.open("wb") as f:
+        f.write(output.getbuffer())
+
+    # Unzip the log archive
+    unzipped_archived_path = tmp_path / "unzipped_folder"
+    unzipped_archived_path.mkdir()
+    with ZipFile(archive_path.as_posix(), mode="r") as zipfile:
+        zipfile.extractall(path=unzipped_archived_path.as_posix())
+
+    # Verify that all expected items are present
+    glob_list = [file.name for file in unzipped_archived_path.rglob("*")]
+    debug(glob_list)
+    assert "file1" in glob_list
+    assert "file2" in glob_list
+    assert "folder" in glob_list
+    assert "file3" in glob_list
+    assert "file4" in glob_list