Repository: fractal-analytics-platform/fractal-client (language: python)

Issue:
Improved log formatting (part 2)
Following up on this issue: https://github.com/fractal-analytics-platform/fractal-server/issues/501

The log formatting still is suboptimal, at least in some cases. The output of `fractal task check-collection 1` when we have a collection failure looks like this:
<img width="1340" alt="Screenshot 2023-02-24 at 10 38 09" src="https://user-images.githubusercontent.com/18033446/221144777-bb236e6f-8552-4b00-8d1f-554d4551b73d.png">

Wasn't fractal-analytics-platform/fractal-server#501 supposed to handle the `\n` formatting?


Discussion / hints:
> Wasn't fractal-analytics-platform/fractal-server#501 supposed to handle the `\n` formatting?

No, it was handling the formatting of task/job errors. This is a task-collection error, coming from different code paths. The fix will likely be similar, but I should look into it
Ah, makes sense! Can "pretty log formatting" be added to all fractal client commands that can return error messages?

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 22128a031..a79f37871 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,7 @@
 * Drop support for python 3.8 (\#438).
 * Update `fractal task edit` command (\#439).
 * Disable `argparse` abbreviation for CLI commands (\#441).
+* Improve task-collection log formatting (\#443).
 
 # 1.0.5
 
diff --git a/fractal/cmd/_task.py b/fractal/cmd/_task.py
index 610eea126..ec317d5fd 100644
--- a/fractal/cmd/_task.py
+++ b/fractal/cmd/_task.py
@@ -55,13 +55,34 @@ async def task_collect_pip(
 
 
 async def task_collection_check(
-    client: AuthClient, *, state_id: int, include_logs: bool, **kwargs
+    client: AuthClient,
+    *,
+    state_id: int,
+    include_logs: bool,
+    do_not_separate_logs: bool = False,
+    **kwargs,
 ) -> BaseInterface:
+
     res = await client.get(
         f"{settings.BASE_URL}/task/collect/{state_id}?verbose={include_logs}"
     )
     state = check_response(res, expected_status_code=200, coerce=StateRead)
-    return RichJsonInterface(retcode=0, data=state.sanitised_dict())
+
+    state_dict = state.sanitised_dict()
+
+    # Remove key-value pairs with None value
+    state_dict["data"] = {
+        key: val for (key, val) in state_dict["data"].items() if val
+    }
+
+    if (not include_logs) or do_not_separate_logs:
+        return RichJsonInterface(retcode=0, data=state_dict)
+    else:
+        log = state_dict["data"].pop("log")
+        extra_lines = ["\nThis is the task-collection log:\n", log]
+        return RichJsonInterface(
+            retcode=0, data=state_dict, extra_lines=extra_lines
+        )
 
 
 async def task_new(
diff --git a/fractal/parser.py b/fractal/parser.py
index 7d47fce12..3444204ad 100644
--- a/fractal/parser.py
+++ b/fractal/parser.py
@@ -290,6 +290,14 @@
     action="store_true",
     help="Also include task-collection logs",
 )
+task_check_collection_parser.add_argument(
+    "--do-not-separate-logs",
+    dest="do_not_separate_logs",
+    help="Show the job logs in the main output, instead of a separate field",
+    action="store_true",
+    required=False,
+)
+
 
 # task new
 task_new_parser = task_subparsers.add_parser(


Test patch:
diff --git a/tests/test_task.py b/tests/test_task.py
index e9516b162..6f78689e7 100644
--- a/tests/test_task.py
+++ b/tests/test_task.py
@@ -42,22 +42,30 @@ async def test_task_collection_and_list(register_user, invoke, testdata_path):
         if res1.data["data"]["status"] == "OK":
             break
         assert time.perf_counter() - starting_time < COLLECTION_TIMEOUT
-    assert res1.data["data"]["log"] is None
+    assert "log" not in res1.data["data"].keys()
 
-    # Add --include-logs flag
-    res2 = await invoke(f"task check-collection {state_id} --include-logs")
+    # Add --include-logs and --do-not-separate-logs flags
+    res2 = await invoke(
+        f"task check-collection {state_id}"
+        " --include-logs"
+        " --do-not-separate-logs"
+    )
     assert res2.retcode == 0
     res2.show()
-    assert res2.data["data"]["log"] is not None
+    assert res2.data["data"]["log"]
     assert res2.data["data"]["status"] == "OK"
 
-    # LIST
-
+    # List tasks
     res = await invoke("task list")
     res.show()
     assert res.retcode == 0
     assert len(res.data) == 2
 
+    # Show again the check-collection output, without --do-not-separate-logs,
+    # for visual inspection
+    res = await invoke(f"task check-collection {state_id} --include-logs")
+    res.show()
+
 
 async def test_repeated_task_collection(register_user, invoke, testdata_path):
     """