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

Issue:
Update `task edit` command
After https://github.com/fractal-analytics-platform/fractal-server/issues/192 is done
Update `task edit` command
After https://github.com/fractal-analytics-platform/fractal-server/issues/192 is done


Discussion / hints:
The CLI parser should be similar `fractal task new`:
```
fractal task new [-h] [--input-type INPUT_TYPE] [--output-type OUTPUT_TYPE]
                 [--default-args-file DEFAULT_ARGS_FILE]
                 [--meta-file META_FILE]
                 name command source
```
but with the "obvious" changes, so that it looks like
```
fractal task edit [-h]
  [--name NAME]
  [--command COMMAND]
  [--input-type INPUT_TYPE]
  [--output-type OUTPUT_TYPE]
  [--default-args-file DEFAULT_ARGS_FILE]
  [--meta-file META_FILE]
  task_id_or_name
```
The CLI parser should be similar `fractal task new`:
```
fractal task new [-h] [--input-type INPUT_TYPE] [--output-type OUTPUT_TYPE]
                 [--default-args-file DEFAULT_ARGS_FILE]
                 [--meta-file META_FILE]
                 name command source
```
but with the "obvious" changes, so that it looks like
```
fractal task edit [-h]
  [--name NAME]
  [--command COMMAND]
  [--input-type INPUT_TYPE]
  [--output-type OUTPUT_TYPE]
  [--default-args-file DEFAULT_ARGS_FILE]
  [--meta-file META_FILE]
  task_id_or_name
```

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5c5bc2ded..11be93a92 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 * Drop support for python 3.8 (\#438).
 * Update `_TaskBase` schema from fractal-common (\#431).
+* Update `fractal task edit` command (\#439).
 
 # 1.0.5
 
diff --git a/fractal/cmd/_task.py b/fractal/cmd/_task.py
index ba2d1376b..610eea126 100644
--- a/fractal/cmd/_task.py
+++ b/fractal/cmd/_task.py
@@ -105,9 +105,31 @@ async def task_edit(
     client: AuthClient,
     *,
     task_id_or_name: str,
-    **task_update_dict,
+    name: Optional[str] = None,
+    command: Optional[str] = None,
+    input_type: Optional[str] = None,
+    output_type: Optional[str] = None,
+    default_args_file: Optional[str] = None,
+    meta_file: Optional[str] = None,
+    **kwargs,
 ) -> BaseInterface:
-    task_update = TaskUpdate(**task_update_dict)
+    update = {}
+    if name:
+        update["name"] = name
+    if command:
+        update["command"] = command
+    if input_type:
+        update["input_type"] = input_type
+    if output_type:
+        update["output_type"] = output_type
+    if default_args_file:
+        with open(default_args_file, "r") as f:
+            update["default_args"] = json.load(f)
+    if meta_file:
+        with open(meta_file, "r") as f:
+            update["meta"] = json.load(f)
+
+    task_update = TaskUpdate(**update)  # validation
     payload = task_update.dict(exclude_unset=True)
     if not payload:
         return PrintInterface(retcode=1, data="Nothing to update")
diff --git a/fractal/parser.py b/fractal/parser.py
index a34b94321..994ea4caf 100644
--- a/fractal/parser.py
+++ b/fractal/parser.py
@@ -295,16 +295,25 @@
     "task_id_or_name", help="ID or name of task to edit", type=str
 )
 task_edit_parser.add_argument("--name", help="New task name")
+task_edit_parser.add_argument("--command", help="New task command")
 task_edit_parser.add_argument(
     "--input-type",
     help="New input type",
 )
 task_edit_parser.add_argument(
     "--output-type",
-    help="New resource type",
+    help="New output type",
 )
 task_edit_parser.add_argument(
-    "--default-args", help="Filename containing JSON encoded default arguments"
+    "--default-args-file",
+    help=(
+        "Path to JSON serialised file containing "
+        "the task default arguments dictionary"
+    ),
+)
+task_edit_parser.add_argument(
+    "--meta-file",
+    help="Path to JSON serialised file containing the task meta dictionary",
 )
 
 # task delete
@@ -395,14 +404,14 @@
     "--args-file",
     help=(
         "Path to json serialised file containing the arguments "
-        "ovverrides of the task"
+        "overrides of the task"
     ),
 )
 workflow_add_task_parser.add_argument(
     "--meta-file",
     help=(
-        "Path to json serialised file containing the meta"
-        "ovverrides of the task"
+        "Path to json serialised file containing the meta "
+        "overrides of the task"
     ),
 )
 
@@ -422,14 +431,14 @@
     "--args-file",
     help=(
         "Path to json serialised file containing the arguments "
-        "ovverrides of the task"
+        "overrides of the task"
     ),
 )
 workflow_edit_task_parser.add_argument(
     "--meta-file",
     help=(
         "Path to json serialised file containing the meta "
-        "ovverrides of the task"
+        "overrides of the task"
     ),
 )
 
diff --git a/poetry.lock b/poetry.lock
index 0086cab63..4105bcef0 100644
--- a/poetry.lock
+++ b/poetry.lock
@@ -490,8 +490,6 @@ files = [
     {file = "cryptography-39.0.1-cp36-abi3-win32.whl", hash = "sha256:fe913f20024eb2cb2f323e42a64bdf2911bb9738a15dba7d3cce48151034e3a8"},
     {file = "cryptography-39.0.1-cp36-abi3-win_amd64.whl", hash = "sha256:ced4e447ae29ca194449a3f1ce132ded8fcab06971ef5f618605aacaa612beac"},
     {file = "cryptography-39.0.1-pp38-pypy38_pp73-macosx_10_12_x86_64.whl", hash = "sha256:807ce09d4434881ca3a7594733669bd834f5b2c6d5c7e36f8c00f691887042ad"},
-    {file = "cryptography-39.0.1-pp38-pypy38_pp73-manylinux_2_28_aarch64.whl", hash = "sha256:c5caeb8188c24888c90b5108a441c106f7faa4c4c075a2bcae438c6e8ca73cef"},
-    {file = "cryptography-39.0.1-pp38-pypy38_pp73-manylinux_2_28_x86_64.whl", hash = "sha256:4789d1e3e257965e960232345002262ede4d094d1a19f4d3b52e48d4d8f3b885"},
     {file = "cryptography-39.0.1-pp38-pypy38_pp73-win_amd64.whl", hash = "sha256:96f1157a7c08b5b189b16b47bc9db2332269d6680a196341bf30046330d15388"},
     {file = "cryptography-39.0.1-pp39-pypy39_pp73-macosx_10_12_x86_64.whl", hash = "sha256:e422abdec8b5fa8462aa016786680720d78bdce7a30c652b7fadf83a4ba35336"},
     {file = "cryptography-39.0.1-pp39-pypy39_pp73-manylinux_2_17_aarch64.manylinux2014_aarch64.manylinux_2_24_aarch64.whl", hash = "sha256:b0afd054cd42f3d213bf82c629efb1ee5f22eba35bf0eec88ea9ea7304f511a2"},
@@ -678,11 +676,11 @@ sqlmodel = "*"
 
 [[package]]
 name = "fractal-server"
-version = "1.0.7"
+version = "1.0.8"
 description = "Server component of the Fractal analytics platform"
 category = "dev"
 optional = false
-python-versions = "^3.8"
+python-versions = "^3.9"
 files = []
 develop = false
 
@@ -707,7 +705,7 @@ slurm = ["clusterfutures (>=0.5,<0.6)"]
 type = "git"
 url = "https://github.com/fractal-analytics-platform/fractal-server.git"
 reference = "main"
-resolved_reference = "e0ca9c0aa35fa4468c10524fc7df5a7b9d2b99a8"
+resolved_reference = "5c57dd5c440cfaf97454864b396e5b634f8f2aef"
 
 [[package]]
 name = "greenlet"
@@ -1779,7 +1777,7 @@ greenlet = {version = "!=0.4.17", markers = "python_version >= \"3\" and (platfo
 
 [package.extras]
 aiomysql = ["aiomysql", "greenlet (!=0.4.17)"]
-aiosqlite = ["aiosqlite", "greenlet (!=0.4.17)", "typing-extensions (!=3.10.0.1)"]
+aiosqlite = ["aiosqlite", "greenlet (!=0.4.17)", "typing_extensions (!=3.10.0.1)"]
 asyncio = ["greenlet (!=0.4.17)"]
 asyncmy = ["asyncmy (>=0.2.3,!=0.2.4)", "greenlet (!=0.4.17)"]
 mariadb-connector = ["mariadb (>=1.0.1)"]
@@ -1789,14 +1787,14 @@ mssql-pyodbc = ["pyodbc"]
 mypy = ["mypy (>=0.910)", "sqlalchemy2-stubs"]
 mysql = ["mysqlclient (>=1.4.0)", "mysqlclient (>=1.4.0,<2)"]
 mysql-connector = ["mysql-connector-python"]
-oracle = ["cx-oracle (>=7)", "cx-oracle (>=7,<8)"]
+oracle = ["cx_oracle (>=7)", "cx_oracle (>=7,<8)"]
 postgresql = ["psycopg2 (>=2.7)"]
 postgresql-asyncpg = ["asyncpg", "greenlet (!=0.4.17)"]
 postgresql-pg8000 = ["pg8000 (>=1.16.6)"]
 postgresql-psycopg2binary = ["psycopg2-binary"]
 postgresql-psycopg2cffi = ["psycopg2cffi"]
 pymysql = ["pymysql", "pymysql (<1)"]
-sqlcipher = ["sqlcipher3-binary"]
+sqlcipher = ["sqlcipher3_binary"]
 
 [[package]]
 name = "sqlalchemy-utils"


Test patch:
diff --git a/tests/data/task_edit_json/default_args.json b/tests/data/task_edit_json/default_args.json
new file mode 100644
index 000000000..dc86b2470
--- /dev/null
+++ b/tests/data/task_edit_json/default_args.json
@@ -0,0 +1,6 @@
+{
+	"Lorem": "ipsum",
+	"dolor": ["sit", "amet"],
+	"consectetur": "adipiscing elit",
+	"sed": 123
+}
diff --git a/tests/data/task_edit_json/meta.json b/tests/data/task_edit_json/meta.json
new file mode 100644
index 000000000..d8198e260
--- /dev/null
+++ b/tests/data/task_edit_json/meta.json
@@ -0,0 +1,5 @@
+{
+		"ut": ["labore", 3.14],
+		"et": "dolore magna",
+		"aliqua": null
+}
diff --git a/tests/test_task.py b/tests/test_task.py
index 1dff64d08..e9516b162 100644
--- a/tests/test_task.py
+++ b/tests/test_task.py
@@ -1,5 +1,6 @@
 import asyncio
 import time
+from pathlib import Path
 
 import pytest
 from devtools import debug
@@ -93,13 +94,6 @@ async def test_repeated_task_collection(register_user, invoke, testdata_path):
     assert res0.data["data"]["info"] == "Already installed"
 
 
-@pytest.mark.xfail
-async def test_edit_task(register_user, invoke, clear_task_cache):
-    # TODO:
-    # Decide what it means to edit a task
-    raise NotImplementedError
-
-
 async def test_task_new(register_user, invoke):
 
     # create a new task with just positional required args
@@ -128,3 +122,58 @@ async def test_task_new(register_user, invoke):
     # create a new task passing not existing file
     with pytest.raises(FileNotFoundError):
         await invoke("task new _name _command _source --meta-file ./foo.pdf")
+
+
+async def test_task_edit(
+    register_user,
+    invoke,
+    invoke_as_superuser,
+    testdata_path: Path,
+):
+    task = await invoke("task new _name _command _source")
+    task.show()
+    assert task.retcode == 0
+    task_id = task.data["id"]
+    NEW = "new"
+
+    # Test that regular user is not authorized
+    with pytest.raises(SystemExit):
+        res = await invoke(f"task edit {task_id} --name {NEW}")
+
+    # Test successful edit of string attributes
+    res = await invoke_as_superuser(f"task edit {task_id} --name {NEW}")
+    assert res.data["name"] == NEW
+    assert res.retcode == 0
+    res = await invoke_as_superuser(f"task edit {task_id} --command {NEW}")
+    assert res.data["command"] == NEW
+    assert res.retcode == 0
+    res = await invoke_as_superuser(f"task edit {task_id} --input-type {NEW}")
+    assert res.data["input_type"] == NEW
+    assert res.retcode == 0
+    res = await invoke_as_superuser(f"task edit {task_id} --output-type {NEW}")
+    assert res.data["output_type"] == NEW
+    assert res.retcode == 0
+
+    # Test `file not found` errors
+    with pytest.raises(FileNotFoundError):
+        res = await invoke_as_superuser(
+            f"task edit {task_id} --default-args-file {NEW}"
+        )
+    with pytest.raises(FileNotFoundError):
+        await invoke_as_superuser(f"task edit {task_id} --meta-file {NEW}")
+
+    # Test successful edit of dictionary attributes
+    args_file = str(testdata_path / "task_edit_json/default_args.json")
+    res = await invoke_as_superuser(
+        f"task edit {task_id} --default-args-file {args_file}"
+    )
+    debug(res)
+    debug(res.data)
+    assert res.retcode == 0
+    meta_file = str(testdata_path / "task_edit_json/meta.json")
+    res = await invoke_as_superuser(
+        f"task edit {task_id} --meta-file {meta_file}"
+    )
+    debug(res)
+    debug(res.data)
+    assert res.retcode == 0
diff --git a/tests/test_user.py b/tests/test_user.py
index edb96fbbc..f3a829e12 100644
--- a/tests/test_user.py
+++ b/tests/test_user.py
@@ -13,7 +13,6 @@ async def test_register_as_user(invoke, register_user, caplog):
         await invoke(f"user register {EMAIL_USER} {PWD_USER}")
     debug(caplog.text)
     assert "403" in caplog.text
-    assert "Forbidden" in caplog.text
 
 
 @pytest.mark.parametrize("is_superuser", [True, False])
@@ -55,7 +54,6 @@ async def test_list_as_user(invoke, register_user, caplog):
         await invoke("user list")
     debug(caplog.text)
     assert "403" in caplog.text
-    assert "Forbidden" in caplog.text
 
 
 async def test_list_as_superuser(invoke_as_superuser, register_user):
@@ -79,7 +77,6 @@ async def test_show_as_user(
         await invoke(f"user show {user_id}")
     debug(caplog.text)
     assert "403" in caplog.text
-    assert "Forbidden" in caplog.text
 
 
 async def test_show_as_superuser(invoke_as_superuser):
@@ -104,7 +101,6 @@ async def test_edit_as_user(
         await invoke(f"user edit {user_id} --new-email email@something.xy")
     debug(caplog.text)
     assert "403" in caplog.text
-    assert "Forbidden" in caplog.text
 
 
 @pytest.mark.parametrize("new_is_superuser", [True, False])
@@ -161,7 +157,6 @@ async def test_delete_as_user(
         await invoke(f"user delete {user_id}")
     debug(caplog.text)
     assert "403" in caplog.text
-    assert "Forbidden" in caplog.text
 
 
 async def test_delete_as_superuser(invoke_as_superuser, caplog):