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

Issue:
Rename arguments for task-edit and workflow-edit commands
(this is needed for #345)

Prepend `new` to attribute name, for "edit" commands.
Affected commands:
* `task edit`, apart from `--default-args-file` and `--meta-file` (note: let's clarify the help messages for these arguments)
* `workflow edit` (name --> new-name)
Do not use catch-all `**kwargs` in cmd functions



Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d679dc516..a95847658 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
 * Add `fractal job stop` command (\#485).
 * Specific changes to existing commands:
     * Make `project_id` a required positional argument of `fractal {workflow,dataset,job}` commands (\#479).
+    * For `edit` commands, always prepend the new arguments with `new`, e.g. as in `task edit ... --new-version` (\#498).
     * Add `--type` optional argument to `fractal dataset new` command (\#479).
     * For `fractal workflow apply`:
         * Ttransform `project_id` from option to positional argument (\#479).
diff --git a/fractal/client.py b/fractal/client.py
index 07c255865..0d41ad828 100644
--- a/fractal/client.py
+++ b/fractal/client.py
@@ -87,7 +87,11 @@ async def handle(cli_args: List[str] = argv):
         exit(1)
 
     try:
-        kwargs = vars(args)
+        # Make a copy of vars(args), and remove cmd (which is not a relevant
+        # argument for functions called with **kwargs)
+        kwargs = vars(args).copy()
+        kwargs.pop("cmd")
+
         if args.cmd == "version":
             async with AsyncClient() as client:
                 interface = await handler(client, **kwargs)
diff --git a/fractal/cmd/__init__.py b/fractal/cmd/__init__.py
index 74a424b00..bbbcf6358 100644
--- a/fractal/cmd/__init__.py
+++ b/fractal/cmd/__init__.py
@@ -118,8 +118,13 @@ async def dataset(
 
 
 async def task(
-    client: AuthClient, subcmd: str, batch: bool = False, **kwargs
+    client: AuthClient,
+    subcmd: str,
+    batch: bool = False,
+    verbose: bool = False,
+    **kwargs,
 ) -> BaseInterface:
+
     if subcmd == "list":
         iface = await get_task_list(client)
     elif subcmd == "collect":
@@ -138,7 +143,11 @@ async def task(
 
 
 async def workflow(
-    client: AuthClient, subcmd: str, batch: bool = False, **kwargs
+    client: AuthClient,
+    subcmd: str,
+    batch: bool = False,
+    verbose: bool = False,
+    **kwargs,
 ) -> BaseInterface:
     if subcmd == "show":
         iface = await get_workflow(client, **kwargs)
diff --git a/fractal/cmd/_task.py b/fractal/cmd/_task.py
index ed0052069..bedb06320 100644
--- a/fractal/cmd/_task.py
+++ b/fractal/cmd/_task.py
@@ -132,26 +132,25 @@ async def patch_task(
     client: AuthClient,
     *,
     task_id_or_name: str,
-    name: Optional[str] = None,
-    command: Optional[str] = None,
-    input_type: Optional[str] = None,
-    output_type: Optional[str] = None,
-    version: Optional[str] = None,
+    new_name: Optional[str] = None,
+    new_command: Optional[str] = None,
+    new_input_type: Optional[str] = None,
+    new_output_type: Optional[str] = None,
+    new_version: Optional[str] = None,
     default_args_file: Optional[str] = None,
     meta_file: Optional[str] = None,
-    **kwargs,
 ) -> BaseInterface:
     update = {}
-    if name:
-        update["name"] = name
-    if command:
-        update["command"] = command
-    if version:
-        update["version"] = version
-    if input_type:
-        update["input_type"] = input_type
-    if output_type:
-        update["output_type"] = output_type
+    if new_name:
+        update["name"] = new_name
+    if new_command:
+        update["command"] = new_command
+    if new_version:
+        update["version"] = new_version
+    if new_input_type:
+        update["input_type"] = new_input_type
+    if new_output_type:
+        update["output_type"] = new_output_type
     if default_args_file:
         with open(default_args_file, "r") as f:
             update["default_args"] = json.load(f)
diff --git a/fractal/cmd/_workflow.py b/fractal/cmd/_workflow.py
index e691332cb..65d11b604 100644
--- a/fractal/cmd/_workflow.py
+++ b/fractal/cmd/_workflow.py
@@ -101,7 +101,6 @@ async def post_workflowtask(
     order: Optional[int] = None,
     args_file: Optional[str] = None,
     meta_file: Optional[str] = None,
-    **kwargs,
 ) -> RichJsonInterface:
 
     try:
@@ -148,7 +147,6 @@ async def patch_workflowtask(
     workflow_task_id: int,
     args_file: Optional[str] = None,
     meta_file: Optional[str] = None,
-    **kwargs,
 ) -> RichJsonInterface:
 
     # Check that at least one of args_file or meta_file was given (note: it
@@ -207,12 +205,11 @@ async def patch_workflow(
     *,
     project_id: int,
     workflow_id: int,
-    **workflow_update_dict,
+    new_name: str,
 ) -> BaseInterface:
-    workflow_update = WorkflowUpdate(**workflow_update_dict)
+
+    workflow_update = WorkflowUpdate(name=new_name)
     payload = workflow_update.dict(exclude_unset=True)
-    if not payload:
-        return PrintInterface(retcode=1, data="Nothing to update")
 
     res = await client.patch(
         f"{settings.BASE_URL}/project/{project_id}/workflow/{workflow_id}",
diff --git a/fractal/parser.py b/fractal/parser.py
index d6a30bd69..e78b90b5a 100644
--- a/fractal/parser.py
+++ b/fractal/parser.py
@@ -376,15 +376,17 @@
 task_edit_parser.add_argument(
     "task_id_or_name", help="ID or name of task to edit", type=str
 )
-task_edit_parser.add_argument("--name", help="New task name", type=str)
-task_edit_parser.add_argument("--command", help="New task command", type=str)
+task_edit_parser.add_argument("--new-name", help="New task name", type=str)
 task_edit_parser.add_argument(
-    "--input-type",
+    "--new-command", help="New task command", type=str
+)
+task_edit_parser.add_argument(
+    "--new-input-type",
     type=str,
     help="New input type",
 )
 task_edit_parser.add_argument(
-    "--output-type",
+    "--new-output-type",
     type=str,
     help="New output type",
 )
@@ -392,17 +394,20 @@
     "--default-args-file",
     type=str,
     help=(
-        "Path to JSON serialised file containing "
-        "the task default arguments dictionary"
+        "Path to JSON serialised file containing updates to the current"
+        "`default_args` dictionary"
     ),
 )
 task_edit_parser.add_argument(
     "--meta-file",
     type=str,
-    help="Path to JSON serialised file containing the task meta dictionary",
+    help=(
+        "Path to JSON serialised file containing updates to the current "
+        "`meta` dictionary"
+    ),
 )
 task_edit_parser.add_argument(
-    "--version",
+    "--new-version",
     type=str,
     help="New version",
 )
@@ -470,7 +475,12 @@
 )
 workflow_edit_parser.add_argument("project_id", type=int, help="Project ID")
 workflow_edit_parser.add_argument("workflow_id", type=int, help="Workflow ID")
-workflow_edit_parser.add_argument("--name", type=str, help="New workflow name")
+workflow_edit_parser.add_argument(
+    "--new-name",
+    type=str,
+    help="New workflow name",
+    required=True,
+)
 
 # workflow delete
 workflow_delete_parser = workflow_subparsers.add_parser(


Test patch:
diff --git a/tests/test_task.py b/tests/test_task.py
index 9737bcf73..95cb0a2c0 100644
--- a/tests/test_task.py
+++ b/tests/test_task.py
@@ -176,22 +176,26 @@ async def test_task_edit(
 
     # Test that regular user is not authorized
     with pytest.raises(SystemExit):
-        res = await invoke(f"task edit {task_id} --name {NEW}")
+        res = await invoke(f"task edit {task_id} --new-name {NEW}")
 
     # Test successful edit of string attributes
-    res = await invoke_as_superuser(f"task edit {task_id} --name {NEW}")
+    res = await invoke_as_superuser(f"task edit {task_id} --new-name {NEW}")
     assert res.data["name"] == NEW
     assert res.retcode == 0
-    res = await invoke_as_superuser(f"task edit {task_id} --command {NEW}")
+    res = await invoke_as_superuser(f"task edit {task_id} --new-command {NEW}")
     assert res.data["command"] == NEW
     assert res.retcode == 0
-    res = await invoke_as_superuser(f"task edit {task_id} --input-type {NEW}")
+    res = await invoke_as_superuser(
+        f"task edit {task_id} --new-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}")
+    res = await invoke_as_superuser(
+        f"task edit {task_id} --new-output-type {NEW}"
+    )
     assert res.data["output_type"] == NEW
     assert res.retcode == 0
-    res = await invoke_as_superuser(f"task edit {task_id} --version {NEW}")
+    res = await invoke_as_superuser(f"task edit {task_id} --new-version {NEW}")
     assert res.data["version"] == NEW
     assert res.retcode == 0
 
diff --git a/tests/test_workflow.py b/tests/test_workflow.py
index c69337141..e704d402d 100644
--- a/tests/test_workflow.py
+++ b/tests/test_workflow.py
@@ -62,9 +62,14 @@ async def test_workflow_edit(register_user, invoke):
     workflow_id = res_wf.data["id"]
     assert res_wf.retcode == 0
 
+    # Fail editing with no edits
+    cmd = f"workflow edit {project_id} {workflow_id}"
+    with pytest.raises(SystemExit):
+        res = await invoke(cmd)
+
     # Edit workflow name
     NAME = "new-workflow-name"
-    cmd = f"workflow edit {project_id} {workflow_id} --name {NAME}"
+    cmd = f"workflow edit {project_id} {workflow_id} --new-name {NAME}"
     debug(cmd)
     res_edit = await invoke(cmd)
     assert res_edit.retcode == 0