diff --git a/CHANGELOG.md b/CHANGELOG.md index c3c1142c..ce30d0da 100644 --- a/CHANGELOG.md +++ b/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). diff --git a/fractal_client/cmd/__init__.py b/fractal_client/cmd/__init__.py index 78bfe1b5..71434d2e 100644 --- a/fractal_client/cmd/__init__.py +++ b/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": diff --git a/fractal_client/cmd/_task_collection.py b/fractal_client/cmd/_task_collection.py index 265b1f0d..d97bf1d8 100644 --- a/fractal_client/cmd/_task_collection.py +++ b/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) diff --git a/fractal_client/response.py b/fractal_client/response.py index 60b299a6..4ee2a3f2 100644 --- a/fractal_client/response.py +++ b/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 diff --git a/poetry.lock b/poetry.lock index 643d5b24..6482fbb3 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.4 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand. [[package]] name = "alembic" @@ -697,13 +697,13 @@ pytest = ["pytest (>=7)"] [[package]] name = "fastapi" -version = "0.115.5" +version = "0.115.6" description = "FastAPI framework, high performance, easy to learn, fast to code, ready for production" optional = false python-versions = ">=3.8" files = [ - {file = "fastapi-0.115.5-py3-none-any.whl", hash = "sha256:596b95adbe1474da47049e802f9a65ab2ffa9c2b07e7efee70eb8a66c9f2f796"}, - {file = "fastapi-0.115.5.tar.gz", hash = "sha256:0e7a4d0dc0d01c68df21887cce0945e72d3c48b9f4f79dfe7a7d53aa08fbb289"}, + {file = "fastapi-0.115.6-py3-none-any.whl", hash = "sha256:e9240b29e36fa8f4bb7290316988e90c381e5092e0cbe84e7818cc3713bcf305"}, + {file = "fastapi-0.115.6.tar.gz", hash = "sha256:9ec46f7addc14ea472958a96aae5b5de65f39721a46aaf5705c480d9a8b76654"}, ] [package.dependencies] @@ -759,7 +759,7 @@ typing = ["typing-extensions (>=4.12.2)"] [[package]] name = "fractal-server" -version = "2.9.2" +version = "2.10.0a0" description = "Server component of the Fractal analytics platform" optional = false python-versions = "^3.10" @@ -788,7 +788,7 @@ uvicorn-worker = "^0.2.0" type = "git" url = "https://github.com/fractal-analytics-platform/fractal-server.git" reference = "main" -resolved_reference = "1694ca9781614d82484225b8dd0deb69993b80a6" +resolved_reference = "28472d010d8f88a116a6a0d844ef4728073824cb" [[package]] name = "ghp-import" @@ -1090,13 +1090,13 @@ files = [ [[package]] name = "mako" -version = "1.3.6" +version = "1.3.7" description = "A super-fast templating language that borrows the best ideas from the existing templating languages." optional = false python-versions = ">=3.8" files = [ - {file = "Mako-1.3.6-py3-none-any.whl", hash = "sha256:a91198468092a2f1a0de86ca92690fb0cfc43ca90ee17e15d93662b4c04b241a"}, - {file = "mako-1.3.6.tar.gz", hash = "sha256:9ec3a1583713479fae654f83ed9fa8c9a4c16b7bb0daba0e6bbebff50c0d983d"}, + {file = "Mako-1.3.7-py3-none-any.whl", hash = "sha256:d18f990ad57f800ce8e76cbfb0b74afe471c293517e9f5003ace6dad5aa72c36"}, + {file = "mako-1.3.7.tar.gz", hash = "sha256:20405b1232e0759f0e7d87b01f6bb94fce0761747f1cb876ecf90bd512d0b639"}, ] [package.dependencies] @@ -2084,13 +2084,13 @@ type = ["importlib_metadata (>=7.0.2)", "jaraco.develop (>=7.21)", "mypy (>=1.12 [[package]] name = "six" -version = "1.16.0" +version = "1.17.0" description = "Python 2 and 3 compatibility utilities" optional = false -python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*" +python-versions = "!=3.0.*,!=3.1.*,!=3.2.*,>=2.7" files = [ - {file = "six-1.16.0-py2.py3-none-any.whl", hash = "sha256:8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254"}, - {file = "six-1.16.0.tar.gz", hash = "sha256:1e61c37477a1626458e36f7b1d82aa5c9b094fa4802892072e49de9c60c4c926"}, + {file = "six-1.17.0-py2.py3-none-any.whl", hash = "sha256:4721f391ed90541fddacab5acf947aa0d3dc7d27b2e1e8eda2be8970586c3274"}, + {file = "six-1.17.0.tar.gz", hash = "sha256:ff70335d468e7eb6ec65b95b99d3a2836546063f63acc5171de367e834932a81"}, ] [[package]] diff --git a/tests/fixtures_testserver.py b/tests/fixtures_testserver.py index f6bb2715..7dad4fbc 100644 --- a/tests/fixtures_testserver.py +++ b/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")) diff --git a/tests/test_job.py b/tests/test_job.py index b4c9b1ed..23d2da87 100644 --- a/tests/test_job.py +++ b/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 diff --git a/tests/test_task_collection.py b/tests/test_task_collection.py index ca031df5..5404dc9f 100644 --- a/tests/test_task_collection.py +++ b/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)