From b1d5157f5ad25887af328d6c0057f8da594b9a5b Mon Sep 17 00:00:00 2001 From: Tom Close Date: Thu, 17 Oct 2024 13:10:26 +1100 Subject: [PATCH 1/3] changed command to commands --- pipeline2app/xnat/image.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/pipeline2app/xnat/image.py b/pipeline2app/xnat/image.py index 21b36a0..f83cc06 100644 --- a/pipeline2app/xnat/image.py +++ b/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,8 +21,8 @@ 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 ) @@ -60,7 +60,7 @@ def construct_dockerfile( xnat_command = self.command.make_json() # 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_command, build_dir) self.save_store_config(dockerfile, build_dir, for_localhost=for_localhost) @@ -78,7 +78,7 @@ 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], @@ -95,12 +95,15 @@ 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 command in self.commands: + with open(command_jsons_dir / command.name + ".json", "w") as f: + json.dump(xnat_command, f, indent=" ") + dockerfile.copy( + source=["./xnat_commands" + command.name + ".json"], + destination="/xnat_commands/" + command.name + ".json", + ) def save_store_config( self, dockerfile: DockerRenderer, build_dir: Path, for_localhost: bool = False From c0244bb9cf50c0fc4f128a729175d87e644da840 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 18 Oct 2024 13:23:53 +1100 Subject: [PATCH 2/3] implemented multiple commands --- pipeline2app/xnat/cli/entrypoint.py | 3 ++- pipeline2app/xnat/command.py | 2 +- pipeline2app/xnat/deploy.py | 18 +++++++++---- pipeline2app/xnat/image.py | 26 +++++++++++++------ pipeline2app/xnat/tests/test_app.py | 18 +++++++------ .../xnat/tests/test_cli_xnat_images.py | 2 +- 6 files changed, 45 insertions(+), 24 deletions(-) diff --git a/pipeline2app/xnat/cli/entrypoint.py b/pipeline2app/xnat/cli/entrypoint.py index 5e064ae..0d949f6 100644 --- a/pipeline2app/xnat/cli/entrypoint.py +++ b/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, ) diff --git a/pipeline2app/xnat/command.py b/pipeline2app/xnat/command.py index ff1513f..139aafa 100644 --- a/pipeline2app/xnat/command.py +++ b/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 diff --git a/pipeline2app/xnat/deploy.py b/pipeline2app/xnat/deploy.py index 1ba95e9..dd036b4 100644 --- a/pipeline2app/xnat/deploy.py +++ b/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,6 +35,8 @@ 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 ------- @@ -41,8 +44,13 @@ def install_cs_command( 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: diff --git a/pipeline2app/xnat/image.py b/pipeline2app/xnat/image.py index f83cc06..33055c2 100644 --- a/pipeline2app/xnat/image.py +++ b/pipeline2app/xnat/image.py @@ -27,6 +27,15 @@ class XnatApp(App): # type: ignore[misc] ) # 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_refs(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, @@ -81,7 +90,7 @@ def add_entrypoint(self, dockerfile: DockerRenderer, build_dir: Path) -> None: 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 @@ -97,12 +106,13 @@ def copy_command_refs( """ command_jsons_dir = build_dir / "xnat_commands" command_jsons_dir.mkdir(parents=True, exist_ok=True) - for command in self.commands: - with open(command_jsons_dir / command.name + ".json", "w") as f: + 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=["./xnat_commands" + command.name + ".json"], - destination="/xnat_commands/" + command.name + ".json", + source=[f"./xnat_commands/{cmd_name}.json"], + destination=f"/xnat_commands/{cmd_name}.json", ) def save_store_config( diff --git a/pipeline2app/xnat/tests/test_app.py b/pipeline2app/xnat/tests/test_app.py index 8e56d8d..035e94f 100644 --- a/pipeline2app/xnat/tests/test_app.py +++ b/pipeline2app/xnat/tests/test_app.py @@ -47,7 +47,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": "some.one@an.email.org"}], "docs": { "info_url": "http://concatenate.readthefakedocs.io", @@ -108,7 +108,7 @@ def run_spec( "pipeline2app-xnat", ], }, - "command": bids_command_spec, + "commands": {"bids-test-command": bids_command_spec}, "authors": [ {"name": "Some One Else", "email": "some.oneelse@an.email.org"} ], @@ -212,7 +212,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 +224,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 +247,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,7 +261,7 @@ 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 ) diff --git a/pipeline2app/xnat/tests/test_cli_xnat_images.py b/pipeline2app/xnat/tests/test_cli_xnat_images.py index f1bb726..3b1ddf7 100644 --- a/pipeline2app/xnat/tests/test_cli_xnat_images.py +++ b/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, From 2886961226429dd2721c0d751f1ccaddd36141ef Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 18 Oct 2024 13:25:15 +1100 Subject: [PATCH 3/3] added unittest for multiple commands --- pipeline2app/xnat/tests/test_app.py | 139 ++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/pipeline2app/xnat/tests/test_app.py b/pipeline2app/xnat/tests/test_app.py index 035e94f..5f2460b 100644 --- a/pipeline2app/xnat/tests/test_app.py +++ b/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 ( @@ -268,3 +271,139 @@ def test_xnat_cs_pipeline(xnat_repository, run_spec, run_prefix, work_dir): 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": "some.one@an.email.org"}], + "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