Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add wheel path in task collect command #760

Merged
merged 27 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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).
Expand Down
2 changes: 0 additions & 2 deletions fractal_client/cmd/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ def project(
batch: bool = False,
**kwargs,
) -> Interface:

if subcmd == "new":
parameters = ["name"]
function_kwargs = get_kwargs(parameters, kwargs)
Expand Down Expand Up @@ -129,7 +128,6 @@ def task(
batch: bool = False,
**kwargs,
) -> Interface:

if subcmd == "list":
iface = get_task_list(client)
elif subcmd == "collect":
Expand Down
44 changes: 31 additions & 13 deletions fractal_client/cmd/_task_collection.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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:
Expand All @@ -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"])
Expand All @@ -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)
Expand Down
32 changes: 19 additions & 13 deletions fractal_client/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
26 changes: 13 additions & 13 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion tests/fixtures_testserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"))

Expand Down
2 changes: 1 addition & 1 deletion tests/test_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
26 changes: 10 additions & 16 deletions tests/test_task_collection.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import logging
import sys
import time
from urllib.request import urlopen
Expand All @@ -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(
Expand All @@ -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):
Expand Down Expand Up @@ -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"]
Expand Down Expand Up @@ -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)

Expand Down
Loading