Merge pull request #760 from fractal-analytics-platform/add-wheel-path

Add wheel path in task collect command

CHANGELOG.md

@@ -1,10 +1,10 @@
 **Note**: Numbers like (\#123) point to closed Pull Requests on the fractal repository.
 
-# Unreleased
+# 2.6.0
 
+* Align with new task-collection endpoint in `fractal-server` 2.10.0 (\#760).
 * Update versions of pre-commit hooks and add precommit GitHub Action (\#757).
 
-
 # 2.5.1
 
 * Deprecate user `cache_dir` , to align with [fractal-server 2.9.2](https://github.com/fractal-analytics-platform/fractal-server/blob/main/CHANGELOG.md#292) (\#758).

fractal_client/cmd/__init__.py

@@ -63,7 +63,6 @@ def project(
     batch: bool = False,
     **kwargs,
 ) -> Interface:
-
     if subcmd == "new":
         parameters = ["name"]
         function_kwargs = get_kwargs(parameters, kwargs)
@@ -129,7 +128,6 @@ def task(
     batch: bool = False,
     **kwargs,
 ) -> Interface:
-
     if subcmd == "list":
         iface = get_task_list(client)
     elif subcmd == "collect":

fractal_client/cmd/_task_collection.py

@@ -1,6 +1,7 @@
 import json
 import logging
 import sys
+from pathlib import Path
 
 from fractal_client.authclient import AuthClient
 from fractal_client.config import settings
@@ -19,9 +20,8 @@ def task_collect_pip(
     private: bool = False,
     batch: bool = False,
 ) -> Interface:
-
     # Construct TaskCollectPip object
-    task_collect = dict(package=package)
+    task_collect = dict()
     if package_version:
         task_collect["package_version"] = package_version
     if python_version:
@@ -36,18 +36,37 @@ def task_collect_pip(
                     "'--pinned-dependency PACKAGE_NAME=PACKAGE_VERSION'"
                 )
                 sys.exit(1)
-        task_collect["pinned_package_versions"] = {
-            _name: _version
-            for _name, _version in (p.split("=") for p in pinned_dependency)
-        }
+        task_collect["pinned_package_versions"] = json.dumps(
+            {
+                _name: _version
+                for _name, _version in (
+                    p.split("=") for p in pinned_dependency
+                )
+            }
+        )
 
     is_private = "?private=true" if private else ""
-
-    res = client.post(
-        f"{settings.BASE_URL}/task/collect/pip/{is_private}",
-        json=task_collect,
-    )
-
+    endpoint_url = f"{settings.BASE_URL}/task/collect/pip/{is_private}"
+    if package.endswith(".whl"):
+        with open(package, "rb") as f:
+            file = {
+                "file": (
+                    Path(package).name,
+                    f.read(),
+                    "application/zip",
+                )
+            }
+        res = client.post(
+            endpoint_url,
+            data=task_collect,
+            files=file,
+        )
+    else:
+        task_collect["package"] = package
+        res = client.post(
+            endpoint_url,
+            data=task_collect,
+        )
     task_group_activity = check_response(res, expected_status_code=202)
     if batch:
         return Interface(retcode=0, data=task_group_activity["id"])
@@ -67,7 +86,6 @@ def task_collect_custom(
     private: bool = False,
     batch: bool = False,
 ) -> Interface:
-
     try:
         with open(manifest, "r") as f:
             manifest_dict = json.load(f)

fractal_client/response.py

@@ -45,21 +45,27 @@ def check_response(
 
     logging.debug(f"\nResponse status code:\n    {res.status_code}")
     if res.status_code not in expected_status_codes:
-
         logging.error(f"Server returned {res.status_code}")
-        logging.error(
-            f"Original request: {res._request.method} {res._request.url}"
-        )
-
-        payload = res._request._content.decode("utf-8")
-        if redact_long_payload and len(payload) > 0:
-            payload_dict = json.loads(payload)
-            for key, value in payload_dict.items():
-                if len(str(value)) > LONG_PAYLOAD_VALUE_LIMIT:
-                    payload_dict[key] = "[value too long - redacted]"
-            payload = json.dumps(payload_dict)
 
-        logging.error(f"Original payload: {payload}")
+        # The following block relies on private methods, and it may fail for
+        # unexpected reasons (e.g. it is now aware of the difference between
+        # 'application/json' and 'multipart/form-data' requests, and it will
+        # fail for non-jons requests). For this reason it is within a
+        # broad-scope try/except block.
+        try:
+            logging.error(
+                f"Original request: {res._request.method} {res._request.url}"
+            )
+            payload = res._request._content.decode("utf-8")
+            if redact_long_payload and len(payload) > 0:
+                payload_dict = json.loads(payload)
+                for key, value in payload_dict.items():
+                    if len(str(value)) > LONG_PAYLOAD_VALUE_LIMIT:
+                        payload_dict[key] = "[value too long - redacted]"
+                payload = json.dumps(payload_dict)
+            logging.error(f"Original payload: {payload}")
+        except Exception:
+            logging.info("Could not display original-request information.")
 
         error_msg = data
 

tests/fixtures_testserver.py

@@ -46,7 +46,6 @@ def _run_command(cmd: str) -> str:
 
 @pytest.fixture(scope="session", autouse=True)
 def testserver(tester, tmpdir_factory, request):
-
     FRACTAL_TASK_DIR = str(tmpdir_factory.mktemp("TASKS"))
     FRACTAL_RUNNER_WORKING_BASE_DIR = str(tmpdir_factory.mktemp("JOBS"))
 

tests/test_job.py

@@ -25,7 +25,7 @@ def test_job_submit(
     PACKAGE_PATH = "/tmp/fractal_tasks_mock-0.0.1-py3-none-any.whl"
     urlretrieve(PACKAGE_URL, PACKAGE_PATH)
 
-    res = invoke(f"--batch task collect {PACKAGE_PATH}")
+    res = invoke(f"--batch task collect {PACKAGE_PATH} --private")
     assert res.retcode == 0
     activity_id = res.data
 

tests/test_task_collection.py

@@ -1,4 +1,5 @@
 import json
+import logging
 import sys
 import time
 from urllib.request import urlopen
@@ -7,15 +8,18 @@
 import pytest
 from devtools import debug
 
+logging.getLogger("httpx").setLevel(logging.DEBUG)
+
 
 def test_task_collection_command(invoke, caplog):
     """
     Test that all `task collect` options are correctly parsed and included in
     the the payload for the API request.
     """
+    INVALID_PYTHON_VERSION = "xxx"
     PACKAGE = "devtools"
     PACKAGE_VERSION = "0.11.0"
-    PYTHON_VERSION = "1.2"
+    PYTHON_VERSION = INVALID_PYTHON_VERSION
     PACKAGE_EXTRAS = "a,b,c"
     with pytest.raises(SystemExit):
         invoke(
@@ -28,20 +32,8 @@ def test_task_collection_command(invoke, caplog):
                 "--pinned-dependency pydantic=1.10.0"
             )
         )
-
-    # Check that payload was prepared correctly
-    log_lines = [record.message for record in caplog.records]
-    debug(log_lines)
-    payload_line = next(
-        line for line in log_lines if line.startswith("Original payload: ")
-    )
-    assert payload_line
-    payload = json.loads(payload_line.strip("Original payload: "))
-    debug(payload)
-    assert payload["package"] == PACKAGE
-    assert payload["package_version"] == PACKAGE_VERSION
-    assert payload["package_extras"] == PACKAGE_EXTRAS
-    assert payload["python_version"] == PYTHON_VERSION
+    assert "Server returned 422" in caplog.text
+    assert f"given={INVALID_PYTHON_VERSION}" in caplog.text
 
 
 def test_task_collection_invalid_pinned_dependency(invoke, caplog):
@@ -88,7 +80,8 @@ def test_task_collection(invoke_as_custom_user, user_factory, new_name):
     initial_task_list = len(res.data)
 
     res0 = invoke_as_custom_user(
-        f"task collect --private {PACKAGE_PATH}", **new_user
+        f"task collect --private {PACKAGE_PATH}",
+        **new_user,
     )
     debug(res0.data)
     activity_id = res0.data["id"]
@@ -157,6 +150,7 @@ def test_task_collection_custom(
         f"label {python_interpreter} {manifest}"
     )
     res = invoke_as_custom_user(cmd, **new_user)
+    debug(res.data)
     assert res.retcode == 0
     assert isinstance(res.data, list)