Merge pull request #9 from ArcanaFramework/multiple-commands

Added support for multiple commands

pipeline2app/xnat/cli/entrypoint.py

@@ -36,12 +36,13 @@
 def cs_entrypoint(
     address: str,
     spec_path: Path,
+    command: ty.Optional[str],
     **kwargs: ty.Any,
 ) -> None:
 
     image_spec = XnatApp.load(spec_path)
 
-    image_spec.command.execute(
+    image_spec.command(command).execute(
         address,
         **kwargs,
     )

pipeline2app/xnat/command.py

@@ -17,7 +17,7 @@
 @attrs.define(kw_only=True, auto_attribs=False)
 class XnatCommand(ContainerCommand):  # type: ignore[misc]
 
-    image: ty.Optional[XnatApp] = attrs.field(default=None)
+    image: XnatApp = attrs.field(default=None)
     internal_upload: bool = attrs.field(default=False)
 
     # Hard-code the axes of XNAT commands to be clinical

pipeline2app/xnat/deploy.py

@@ -18,7 +18,8 @@ def install_cs_command(
     enable: bool = False,
     projects_to_enable: ty.Sequence[str] = (),
     replace_existing: bool = False,
-) -> None:
+    command_name: ty.Optional[str] = None,
+) -> int:
     """Installs a new command for the XNAT container service and lanches it on
     the specified session.
 
@@ -34,15 +35,22 @@ def install_cs_command(
         ID of the project to enable the command for
     replace_existing : bool
         Whether to replace existing command with the same name
+    command_name : str, optional
+        the command to install, if an image name is provided instead of a command JSON
 
     Returns
     -------
     cmd_id : int
         the ID of the installed command
     """
     if isinstance(image_name_or_command_json, str):
+        if not command_name:
+            raise ValueError(
+                "If the first argument of the install_cs_command is a string "
+                f"('{image_name_or_command_json}) 'command_name' must be provided"
+            )
         command_json_file = extract_file_from_docker_image(
-            image_name_or_command_json, "/xnat_command.json"
+            image_name_or_command_json, f"/xnat_commands/{command_name}.json"
         )
         if command_json_file is None:
             raise RuntimeError(
@@ -54,8 +62,8 @@ def install_cs_command(
         command_json = image_name_or_command_json
     else:
         raise RuntimeError(
-            f"Unrecognised type of 'image_name_or_command_json' arg: {type(image_name_or_command_json)} "
-            "expected str or dict"
+            "Unrecognised type of 'image_name_or_command_json' arg: "
+            f"{type(image_name_or_command_json)} expected str or dict"
         )
 
     cmd_name = command_json["name"]
@@ -67,7 +75,7 @@ def install_cs_command(
                 xlogin.delete(f"/xapi/commands/{cmd['id']}", accepted_status=[200, 204])
                 logger.info(f"Deleted existing command '{cmd_name}'")
 
-    cmd_id = xlogin.post("/xapi/commands", json=command_json).json()
+    cmd_id: int = xlogin.post("/xapi/commands", json=command_json).json()
 
     # Enable the command globally and in the project
     if enable:

pipeline2app/xnat/image.py

@@ -6,7 +6,7 @@
 import attrs
 from neurodocker.reproenv import DockerRenderer
 from frametree.xnat import XnatViaCS
-from frametree.core.serialize import ClassResolver, ObjectConverter
+from frametree.core.serialize import ClassResolver, ObjectListConverter
 from frametree.core.store import Store
 from pipeline2app.core.image import App
 from .command import XnatCommand
@@ -21,12 +21,21 @@ class XnatApp(App):  # type: ignore[misc]
         "fileformats-medimage-extras",
     )
 
-    command: XnatCommand = attrs.field(
-        converter=ObjectConverter(  # type: ignore[misc]
+    commands: ty.List[XnatCommand] = attrs.field(
+        converter=ObjectListConverter(  # type: ignore[misc]
             XnatCommand
         )  # Change the command type to XnatCommand subclass
     )
 
+    @commands.validator
+    def _validate_commands(
+        self,
+        attribute: attrs.Attribute[ty.List[XnatCommand]],
+        commands: ty.List[XnatCommand],
+    ) -> None:
+        if not commands:
+            raise ValueError("At least one command must be defined within that app")
+
     def construct_dockerfile(
         self,
         build_dir: Path,
@@ -57,16 +66,16 @@ def construct_dockerfile(
 
         dockerfile = super().construct_dockerfile(build_dir, **kwargs)
 
-        xnat_command = self.command.make_json()
+        xnat_commands = [c.make_json() for c in self.commands]
 
         # Copy the generated XNAT commands inside the container for ease of reference
-        self.copy_command_ref(dockerfile, xnat_command, build_dir)
+        self.copy_command_refs(dockerfile, xnat_commands, build_dir)
 
         self.save_store_config(dockerfile, build_dir, for_localhost=for_localhost)
 
         # Convert XNAT command label into string that can by placed inside the
         # Docker label
-        commands_label = json.dumps([xnat_command]).replace("$", r"\$")
+        commands_label = json.dumps(xnat_commands).replace("$", r"\$")
 
         self.add_labels(
             dockerfile,
@@ -78,10 +87,10 @@ def construct_dockerfile(
     def add_entrypoint(self, dockerfile: DockerRenderer, build_dir: Path) -> None:
         pass  # Don't need to add entrypoint as the command line is specified in the command JSON
 
-    def copy_command_ref(
+    def copy_command_refs(
         self,
         dockerfile: DockerRenderer,
-        xnat_command: ty.Dict[str, ty.Any],
+        xnat_commands: ty.List[ty.Dict[str, ty.Any]],
         build_dir: Path,
     ) -> None:
         """Copy the generated command JSON within the Docker image for future reference
@@ -95,12 +104,16 @@ def copy_command_ref(
         build_dir : Path
             path to build directory
         """
-        # Copy command JSON inside dockerfile for ease of reference
-        with open(build_dir / "xnat_command.json", "w") as f:
-            json.dump(xnat_command, f, indent="    ")
-        dockerfile.copy(
-            source=["./xnat_command.json"], destination="/xnat_command.json"
-        )
+        command_jsons_dir = build_dir / "xnat_commands"
+        command_jsons_dir.mkdir(parents=True, exist_ok=True)
+        for xnat_command in xnat_commands:
+            cmd_name = xnat_command["name"]
+            with open(command_jsons_dir / f"{cmd_name}.json", "w") as f:
+                json.dump(xnat_command, f, indent="    ")
+            dockerfile.copy(
+                source=[f"./xnat_commands/{cmd_name}.json"],
+                destination=f"/xnat_commands/{cmd_name}.json",
+            )
 
     def save_store_config(
         self, dockerfile: DockerRenderer, build_dir: Path, for_localhost: bool = False

pipeline2app/xnat/tests/test_app.py

@@ -1,4 +1,6 @@
 from pathlib import Path
+import random
+from copy import deepcopy
 import pytest
 from conftest import (
     TEST_XNAT_DATASET_BLUEPRINTS,
@@ -7,6 +9,7 @@
     FileBP,
     access_dataset,
 )
+from frametree.xnat import Xnat
 from pipeline2app.xnat.image import XnatApp
 from pipeline2app.xnat.command import XnatCommand
 from pipeline2app.xnat.deploy import (
@@ -47,7 +50,7 @@ def run_spec(
                 "package": "1.0",
             },
             "title": "A pipeline to test Pipeline2app's deployment tool",
-            "command": command_spec,
+            "commands": {"concatenate-test": command_spec},
             "authors": [{"name": "Some One", "email": "[email protected]"}],
             "docs": {
                 "info_url": "http://concatenate.readthefakedocs.io",
@@ -108,7 +111,7 @@ def run_spec(
                     "pipeline2app-xnat",
                 ],
             },
-            "command": bids_command_spec,
+            "commands": {"bids-test-command": bids_command_spec},
             "authors": [
                 {"name": "Some One Else", "email": "[email protected]"}
             ],
@@ -212,7 +215,9 @@ def test_xnat_cs_pipeline(xnat_repository, run_spec, run_prefix, work_dir):
     # the fact that the container service test XNAT instance shares the
     # outer Docker socket. Since we build the pipeline image with the same
     # socket there is no need to pull it.
-    xnat_command = image_spec.command.make_json()
+
+    cmd = image_spec.command()
+    xnat_command = cmd.make_json()
 
     launch_inputs = {}
 
@@ -222,11 +227,11 @@ def test_xnat_cs_pipeline(xnat_repository, run_spec, run_prefix, work_dir):
     for pname, pval in params.items():
         launch_inputs[pname] = pval
 
-    if image_spec.command.internal_upload:
+    if cmd.internal_upload:
         # If using internal upload, the output names are fixed
-        output_values = {o: o for o in image_spec.command.output_names}
+        output_values = {o: o for o in cmd.output_names}
     else:
-        output_values = {o: o + "_sink" for o in image_spec.command.output_names}
+        output_values = {o: o + "_sink" for o in cmd.output_names}
         launch_inputs.update(output_values)
 
     with xnat_repository.connection:
@@ -245,7 +250,7 @@ def test_xnat_cs_pipeline(xnat_repository, run_spec, run_prefix, work_dir):
 
         assert status == "Complete", f"Workflow {workflow_id} failed.\n{out_str}"
 
-        access_type = "direct" if image_spec.command.internal_upload else "api"
+        access_type = "direct" if cmd.internal_upload else "api"
 
         assert f"via {access_type} access" in out_str.lower()
 
@@ -259,10 +264,146 @@ def test_xnat_cs_pipeline(xnat_repository, run_spec, run_prefix, work_dir):
                 Path(f).name.lstrip("sub-DEFAULT_")
                 for f in test_xsession.resources[sinked_name].files
             )
-            if image_spec.command.internal_upload:
+            if cmd.internal_upload:
                 reference = sorted(
                     d.rstrip("_sink.txt") + ".txt" for d in deriv.filenames
                 )
             else:
                 reference = sorted(deriv.filenames)
             assert uploaded_files == reference
+
+
+def test_multi_command(xnat_repository: Xnat, tmp_path: Path, run_prefix) -> None:
+
+    bp = TestXnatDatasetBlueprint(
+        dim_lengths=[1, 1, 1],
+        scans=[
+            ScanBP(
+                name="scan1",
+                resources=[FileBP(path="TEXT", datatype=Text, filenames=["file1.txt"])],
+            ),
+            ScanBP(
+                name="scan2",
+                resources=[FileBP(path="TEXT", datatype=Text, filenames=["file2.txt"])],
+            ),
+        ],
+    )
+
+    project_id = run_prefix + "multi_command" + str(hex(random.getrandbits(16)))[2:]
+
+    dataset = bp.make_dataset(
+        dataset_id=project_id,
+        store=xnat_repository,
+        name="",
+    )
+
+    two_dup_spec = dict(
+        name="concatenate",
+        task="pipeline2app.testing.tasks:concatenate",
+        row_frequency=Clinical.session.tostr(),
+        inputs=[
+            {
+                "name": "first_file",
+                "datatype": "text/text-file",
+                "field": "in_file1",
+                "help": "dummy",
+            },
+            {
+                "name": "second_file",
+                "datatype": "text/text-file",
+                "field": "in_file2",
+                "help": "dummy",
+            },
+        ],
+        outputs=[
+            {
+                "name": "concatenated",
+                "datatype": "text/text-file",
+                "field": "out_file",
+                "help": "dummy",
+            }
+        ],
+        parameters={
+            "duplicates": {
+                "datatype": "field/integer",
+                "default": 2,
+                "help": "dummy",
+            }
+        },
+    )
+
+    three_dup_spec = deepcopy(two_dup_spec)
+    three_dup_spec["parameters"]["duplicates"]["default"] = 3
+
+    test_spec = {
+        "name": "test_multi_commands",
+        "title": "a test image for multi-image commands",
+        "commands": {
+            "two_duplicates": two_dup_spec,
+            "three_duplicates": three_dup_spec,
+        },
+        "version": {"package": "1.0", "build": "1"},
+        "packages": {
+            "system": ["vim"],  # just to test it out
+            "pip": {
+                "fileformats": None,
+                "pipeline2app": None,
+                "frametree": None,
+            },
+        },
+        "authors": [{"name": "Some One", "email": "[email protected]"}],
+        "docs": {
+            "info_url": "http://concatenate.readthefakedocs.io",
+        },
+    }
+
+    app = XnatApp.load(test_spec)
+
+    app.make(
+        build_dir=tmp_path / "build-dir",
+        pipeline2app_install_extras=["test"],
+        use_local_packages=True,
+        for_localhost=True,
+    )
+
+    fnames = ["file1.txt", "file2.txt"]
+
+    base_launch_inputs = {
+        "first_file": "scan1",
+        "second_file": "scan2",
+    }
+
+    command_names = ["two_duplicates", "three_duplicates"]
+
+    with xnat_repository.connection as xlogin:
+
+        test_xsession = next(iter(xlogin.projects[dataset.id].experiments.values()))
+        for command_name in command_names:
+
+            launch_inputs = deepcopy(base_launch_inputs)
+            launch_inputs["concatenated"] = command_name
+
+            workflow_id, status, out_str = install_and_launch_xnat_cs_command(
+                command_json=app.command(command_name).make_json(),
+                project_id=project_id,
+                session_id=test_xsession.id,
+                inputs=launch_inputs,
+                xlogin=xlogin,
+            )
+
+            assert status == "Complete", f"Workflow {workflow_id} failed.\n{out_str}"
+
+        assert sorted(r.label for r in test_xsession.resources.values()) == sorted(
+            command_names
+        )
+
+    # Add source column to saved dataset
+    reloaded = dataset.reload()
+    for command_name in command_names:
+        sink = reloaded[command_name]
+        duplicates = 2 if command_name == "two_duplicates" else 3
+        expected_contents = "\n".join(fnames * duplicates)
+        for item in sink:
+            with open(item) as f:
+                contents = f.read()
+            assert contents == expected_contents

pipeline2app/xnat/tests/test_cli_xnat_images.py

@@ -52,7 +52,7 @@ def test_deploy_pipelines(
 
         image_spec = {
             "title": "a command to test build process",
-            "command": cmd_spec,
+            "commands": {"test-command": cmd_spec},
             "version": {
                 "package": PKG_VERSION,
                 "build": WRAPPER_VERSION,