From: Peter Amstutz Date: Thu, 19 May 2022 13:45:37 +0000 (-0400) Subject: Merge branch '17004-properties-on-output' refs #17004 X-Git-Tag: 2.5.0~165 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/c36ec856598f214e340e3335ddd347d131335bf8?hp=8b70a18b37ae6f5d081d469af6bcdc8ec3507e2b Merge branch '17004-properties-on-output' refs #17004 Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- diff --git a/doc/api/methods/container_requests.html.textile.liquid b/doc/api/methods/container_requests.html.textile.liquid index 8704701105..15fa207b1c 100644 --- a/doc/api/methods/container_requests.html.textile.liquid +++ b/doc/api/methods/container_requests.html.textile.liquid @@ -61,6 +61,7 @@ table(table table-bordered table-condensed). |runtime_user_uuid|string|The user permission that will be granted to this container.|| |runtime_auth_scopes|array of string|The scopes associated with the auth token used to run this container.|| |output_storage_classes|array of strings|The storage classes that will be used for the log and output collections of this container request|default is ["default"]| +|output_properties|hash|User metadata properties to set on the output collection. The output collection will also have default properties "type" ("intermediate" or "output") and "container_request" (the uuid of container request that produced the collection).| h2(#priority). Priority diff --git a/doc/api/methods/containers.html.textile.liquid b/doc/api/methods/containers.html.textile.liquid index 18fb4f0133..76e5730c9f 100644 --- a/doc/api/methods/containers.html.textile.liquid +++ b/doc/api/methods/containers.html.textile.liquid @@ -60,6 +60,7 @@ Generally this will contain additional keys that are not present in any correspo |gateway_address|string|Address (host:port) of gateway server.|Internal use only.| |interactive_session_started|boolean|Indicates whether @arvados-client shell@ has been used to run commands in the container, which may have altered the container's behavior and output.|| |output_storage_classes|array of strings|The storage classes that will be used for the log and output collections of this container|| +|output_properties|hash|User metadata properties to set on the output collection.| h2(#container_states). Container states diff --git a/doc/user/cwl/cwl-extensions.html.textile.liquid b/doc/user/cwl/cwl-extensions.html.textile.liquid index 0e97e07da3..197816f4a4 100644 --- a/doc/user/cwl/cwl-extensions.html.textile.liquid +++ b/doc/user/cwl/cwl-extensions.html.textile.liquid @@ -58,6 +58,11 @@ hints: property1: value1 property2: $(inputs.value2) + arv:OutputCollectionProperties: + outputProperties: + property1: value1 + property2: $(inputs.value2) + cwltool:CUDARequirement: cudaVersionMin: "11.0" cudaComputeCapability: "9.0" @@ -154,7 +159,15 @@ Specify extra "properties":{{site.baseurl}}/api/methods.html#subpropertyfilters table(table table-bordered table-condensed). |_. Field |_. Type |_. Description | -|processProperties|key-value map, or list of objects with the fields {propertyName, propertyValue}|The properties that will be set on the container request. May include expressions that reference `$(inputs)` of the current workflow or tool.| +|processProperties|key-value map, or list of objects with the fields {propertyName, propertyValue}|The properties that will be set on the container request. May include expressions that reference @$(inputs)@ of the current workflow or tool.| + +h2(#OutputCollectionProperties). arv:OutputCollectionProperties + +Specify custom "properties":{{site.baseurl}}/api/methods.html#subpropertyfilters that will be set on the output collection of the workflow step. + +table(table table-bordered table-condensed). +|_. Field |_. Type |_. Description | +|outputProperties|key-value map, or list of objects with the fields {propertyName, propertyValue}|The properties that will be set on the output collection. May include expressions that reference @$(inputs)@ of the current workflow or tool.| h2(#CUDARequirement). cwltool:CUDARequirement diff --git a/sdk/cwl/arvados_cwl/__init__.py b/sdk/cwl/arvados_cwl/__init__.py index 21b629f37a..08a05d571c 100644 --- a/sdk/cwl/arvados_cwl/__init__.py +++ b/sdk/cwl/arvados_cwl/__init__.py @@ -265,6 +265,7 @@ def add_arv_hints(): "http://arvados.org/cwl#ProcessProperties", "http://commonwl.org/cwltool#CUDARequirement", "http://arvados.org/cwl#UsePreemptible", + "http://arvados.org/cwl#OutputCollectionProperties", ]) def exit_signal_handler(sigcode, frame): diff --git a/sdk/cwl/arvados_cwl/arv-cwl-schema-v1.0.yml b/sdk/cwl/arvados_cwl/arv-cwl-schema-v1.0.yml index af75481431..54e0fc5122 100644 --- a/sdk/cwl/arvados_cwl/arv-cwl-schema-v1.0.yml +++ b/sdk/cwl/arvados_cwl/arv-cwl-schema-v1.0.yml @@ -299,8 +299,8 @@ $graph: - type: record name: PropertyDef doc: | - Define a property that will be set on the submitted container - request associated with this workflow or step. + Define an arvados metadata property that will be set on a + container request or output collection. fields: - name: propertyName type: string @@ -400,3 +400,23 @@ $graph: _id: "@type" _type: "@vocab" usePreemptible: boolean + +- name: OutputCollectionProperties + type: record + extends: cwl:ProcessRequirement + inVocab: false + doc: | + Specify metadata properties that will be set on the output + collection associated with this workflow or step. + fields: + class: + type: string + doc: "Always 'arv:OutputCollectionProperties" + jsonldPredicate: + _id: "@type" + _type: "@vocab" + outputProperties: + type: PropertyDef[] + jsonldPredicate: + mapSubject: propertyName + mapPredicate: propertyValue diff --git a/sdk/cwl/arvados_cwl/arv-cwl-schema-v1.1.yml b/sdk/cwl/arvados_cwl/arv-cwl-schema-v1.1.yml index 0ae451ccaa..b60d0ab1c9 100644 --- a/sdk/cwl/arvados_cwl/arv-cwl-schema-v1.1.yml +++ b/sdk/cwl/arvados_cwl/arv-cwl-schema-v1.1.yml @@ -343,3 +343,23 @@ $graph: _id: "@type" _type: "@vocab" usePreemptible: boolean + +- name: OutputCollectionProperties + type: record + extends: cwl:ProcessRequirement + inVocab: false + doc: | + Specify metadata properties that will be set on the output + collection associated with this workflow or step. + fields: + class: + type: string + doc: "Always 'arv:OutputCollectionProperties" + jsonldPredicate: + _id: "@type" + _type: "@vocab" + outputProperties: + type: PropertyDef[] + jsonldPredicate: + mapSubject: propertyName + mapPredicate: propertyValue diff --git a/sdk/cwl/arvados_cwl/arv-cwl-schema-v1.2.yml b/sdk/cwl/arvados_cwl/arv-cwl-schema-v1.2.yml index de5e55ca01..2769244a5d 100644 --- a/sdk/cwl/arvados_cwl/arv-cwl-schema-v1.2.yml +++ b/sdk/cwl/arvados_cwl/arv-cwl-schema-v1.2.yml @@ -345,3 +345,23 @@ $graph: _id: "@type" _type: "@vocab" usePreemptible: boolean + +- name: OutputCollectionProperties + type: record + extends: cwl:ProcessRequirement + inVocab: false + doc: | + Specify metadata properties that will be set on the output + collection associated with this workflow or step. + fields: + class: + type: string + doc: "Always 'arv:OutputCollectionProperties" + jsonldPredicate: + _id: "@type" + _type: "@vocab" + outputProperties: + type: PropertyDef[] + jsonldPredicate: + mapSubject: propertyName + mapPredicate: propertyValue diff --git a/sdk/cwl/arvados_cwl/arvcontainer.py b/sdk/cwl/arvados_cwl/arvcontainer.py index 5082cc2f4b..f3e122e603 100644 --- a/sdk/cwl/arvados_cwl/arvcontainer.py +++ b/sdk/cwl/arvados_cwl/arvcontainer.py @@ -146,6 +146,8 @@ class ArvadosContainer(JobBase): mounts[targetdir]["path"] = path prevdir = targetdir + "/" + intermediate_collection_info = arvados_cwl.util.get_intermediate_collection_info(self.name, runtimeContext.current_container, runtimeContext.intermediate_output_ttl) + with Perf(metrics, "generatefiles %s" % self.name): if self.generatefiles["listing"]: vwd = arvados.collection.Collection(api_client=self.arvrunner.api, @@ -197,12 +199,11 @@ class ArvadosContainer(JobBase): if not runtimeContext.current_container: runtimeContext.current_container = arvados_cwl.util.get_current_container(self.arvrunner.api, self.arvrunner.num_retries, logger) - info = arvados_cwl.util.get_intermediate_collection_info(self.name, runtimeContext.current_container, runtimeContext.intermediate_output_ttl) - vwd.save_new(name=info["name"], + vwd.save_new(name=intermediate_collection_info["name"], owner_uuid=runtimeContext.project_uuid, ensure_unique_name=True, - trash_at=info["trash_at"], - properties=info["properties"]) + trash_at=intermediate_collection_info["trash_at"], + properties=intermediate_collection_info["properties"]) prev = None for f, p in sorteditems: @@ -319,7 +320,7 @@ class ArvadosContainer(JobBase): if runtimeContext.submit_runner_cluster: extra_submit_params["cluster_id"] = runtimeContext.submit_runner_cluster - container_request["output_name"] = "Output for step %s" % (self.name) + container_request["output_name"] = "Output from step %s" % (self.name) container_request["output_ttl"] = self.output_ttl container_request["mounts"] = mounts container_request["secret_mounts"] = secret_mounts @@ -341,6 +342,16 @@ class ArvadosContainer(JobBase): for pr in properties_req["processProperties"]: container_request["properties"][pr["propertyName"]] = self.builder.do_eval(pr["propertyValue"]) + output_properties_req, _ = self.get_requirement("http://arvados.org/cwl#OutputCollectionProperties") + if output_properties_req: + if self.arvrunner.api._rootDesc["revision"] >= "20220510": + container_request["output_properties"] = {} + for pr in output_properties_req["outputProperties"]: + container_request["output_properties"][pr["propertyName"]] = self.builder.do_eval(pr["propertyValue"]) + else: + logger.warning("%s API revision is %s, revision %s is required to support setting properties on output collections.", + self.arvrunner.label(self), self.arvrunner.api._rootDesc["revision"], "20220510") + if runtimeContext.runnerjob.startswith("arvwf:"): wfuuid = runtimeContext.runnerjob[6:runtimeContext.runnerjob.index("#")] wfrecord = self.arvrunner.api.workflows().get(uuid=wfuuid).execute(num_retries=self.arvrunner.num_retries) diff --git a/sdk/cwl/arvados_cwl/executor.py b/sdk/cwl/arvados_cwl/executor.py index 1759e4ac28..fe078e3227 100644 --- a/sdk/cwl/arvados_cwl/executor.py +++ b/sdk/cwl/arvados_cwl/executor.py @@ -32,7 +32,7 @@ from arvados.errors import ApiError import arvados_cwl.util from .arvcontainer import RunnerContainer -from .runner import Runner, upload_docker, upload_job_order, upload_workflow_deps +from .runner import Runner, upload_docker, upload_job_order, upload_workflow_deps, make_builder from .arvtool import ArvadosCommandTool, validate_cluster_target, ArvadosExpressionTool from .arvworkflow import ArvadosWorkflow, upload_workflow from .fsaccess import CollectionFsAccess, CollectionFetcher, collectionResolver, CollectionCache, pdh_size @@ -404,7 +404,7 @@ The 'jobs' API is no longer supported. with SourceLine(obj, i, UnsupportedRequirement, logger.isEnabledFor(logging.DEBUG)): self.check_features(v, parentfield=parentfield) - def make_output_collection(self, name, storage_classes, tagsString, outputObj): + def make_output_collection(self, name, storage_classes, tagsString, output_properties, outputObj): outputObj = copy.deepcopy(outputObj) files = [] @@ -456,7 +456,9 @@ The 'jobs' API is no longer supported. res = str(json.dumps(outputObj, sort_keys=True, indent=4, separators=(',',': '), ensure_ascii=False)) f.write(res) - final.save_new(name=name, owner_uuid=self.project_uuid, storage_classes=storage_classes, ensure_unique_name=True) + + final.save_new(name=name, owner_uuid=self.project_uuid, storage_classes=storage_classes, + ensure_unique_name=True, properties=output_properties) logger.info("Final output collection %s \"%s\" (%s)", final.portable_data_hash(), final.api_response()["name"], @@ -486,6 +488,7 @@ The 'jobs' API is no longer supported. self.api.containers().update(uuid=current['uuid'], body={ 'output': self.final_output_collection.portable_data_hash(), + 'output_properties': self.final_output_collection.get_properties(), }).execute(num_retries=self.num_retries) self.api.collections().update(uuid=self.final_output_collection.manifest_locator(), body={ @@ -624,6 +627,9 @@ The 'jobs' API is no longer supported. runtimeContext.tmpdir_prefix = "tmp" runtimeContext.work_api = self.work_api + if not self.output_name: + self.output_name = "Output from workflow %s" % runtimeContext.name + if self.work_api == "containers": if self.ignore_docker_for_reuse: raise Exception("--ignore-docker-for-reuse not supported with containers API.") @@ -776,8 +782,6 @@ The 'jobs' API is no longer supported. if workbench2 or workbench1: logger.info("Output at %scollections/%s", workbench2 or workbench1, tool.final_output) else: - if self.output_name is None: - self.output_name = "Output of %s" % (shortname(tool.tool["id"])) if self.output_tags is None: self.output_tags = "" @@ -788,7 +792,16 @@ The 'jobs' API is no longer supported. else: storage_classes = runtimeContext.storage_classes.strip().split(",") - self.final_output, self.final_output_collection = self.make_output_collection(self.output_name, storage_classes, self.output_tags, self.final_output) + output_properties = {} + output_properties_req, _ = tool.get_requirement("http://arvados.org/cwl#OutputCollectionProperties") + if output_properties_req: + builder = make_builder(job_order, tool.hints, tool.requirements, runtimeContext, tool.metadata) + for pr in output_properties_req["outputProperties"]: + output_properties[pr["propertyName"]] = builder.do_eval(pr["propertyValue"]) + + self.final_output, self.final_output_collection = self.make_output_collection(self.output_name, storage_classes, + self.output_tags, output_properties, + self.final_output) self.set_crunch_output() if runtimeContext.compute_checksum: diff --git a/sdk/cwl/arvados_cwl/util.py b/sdk/cwl/arvados_cwl/util.py index 85ae65ecf1..a0dfb290c1 100644 --- a/sdk/cwl/arvados_cwl/util.py +++ b/sdk/cwl/arvados_cwl/util.py @@ -16,9 +16,9 @@ def get_intermediate_collection_info(workflow_step_name, current_container, inte if intermediate_output_ttl > 0: trash_time = datetime.datetime.utcnow() + datetime.timedelta(seconds=intermediate_output_ttl) container_uuid = None + props = {"type": "intermediate"} if current_container: - container_uuid = current_container['uuid'] - props = {"type": "intermediate", "container": container_uuid} + props["container"] = current_container['uuid'] return {"name" : name, "trash_at" : trash_time, "properties" : props} diff --git a/sdk/cwl/tests/17004-output-props.cwl b/sdk/cwl/tests/17004-output-props.cwl new file mode 100644 index 0000000000..4cf03bade8 --- /dev/null +++ b/sdk/cwl/tests/17004-output-props.cwl @@ -0,0 +1,22 @@ +# Copyright (C) The Arvados Authors. All rights reserved. +# +# SPDX-License-Identifier: Apache-2.0 + +class: Workflow +cwlVersion: v1.2 +$namespaces: + arv: "http://arvados.org/cwl#" +hints: + arv:OutputCollectionProperties: + outputProperties: + foo: bar + baz: $(inputs.inp.basename) +inputs: + inp: File +steps: + cat: + in: + inp: inp + run: cat.cwl + out: [] +outputs: [] diff --git a/sdk/cwl/tests/arvados-tests.sh b/sdk/cwl/tests/arvados-tests.sh index 7d27523d30..b1a1837466 100755 --- a/sdk/cwl/tests/arvados-tests.sh +++ b/sdk/cwl/tests/arvados-tests.sh @@ -36,5 +36,9 @@ arvados-cwl-runner 18888-download_def.cwl --scripts scripts/ # integration test to check for the expected behavior. $python test_copy_deps.py +# Test for #17004 +# Checks that the final output collection has the expected properties. +python test_set_output_prop.py + # Run integration tests exec cwltest --test arvados-tests.yml --tool arvados-cwl-runner $@ -- --disable-reuse --compute-checksum --api=containers diff --git a/sdk/cwl/tests/test_container.py b/sdk/cwl/tests/test_container.py index 975fcdf8a3..cb57b446da 100644 --- a/sdk/cwl/tests/test_container.py +++ b/sdk/cwl/tests/test_container.py @@ -178,7 +178,7 @@ class TestContainer(unittest.TestCase): "capacity": 1073741824 } }, 'state': 'Committed', - 'output_name': 'Output for step test_run_'+str(enable_reuse), + 'output_name': 'Output from step test_run_'+str(enable_reuse), 'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz', 'output_path': '/var/spool/cwl', 'output_ttl': 0, @@ -267,7 +267,7 @@ class TestContainer(unittest.TestCase): "capacity": 5242880000 } }, 'state': 'Committed', - 'output_name': 'Output for step test_resource_requirements', + 'output_name': 'Output from step test_resource_requirements', 'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz', 'output_path': '/var/spool/cwl', 'output_ttl': 7200, @@ -401,7 +401,7 @@ class TestContainer(unittest.TestCase): } }, 'state': 'Committed', - 'output_name': 'Output for step test_initial_work_dir', + 'output_name': 'Output from step test_initial_work_dir', 'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz', 'output_path': '/var/spool/cwl', 'output_ttl': 0, @@ -489,7 +489,7 @@ class TestContainer(unittest.TestCase): }, }, 'state': 'Committed', - "output_name": "Output for step test_run_redirect", + "output_name": "Output from step test_run_redirect", 'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz', 'output_path': '/var/spool/cwl', 'output_ttl': 0, @@ -725,7 +725,7 @@ class TestContainer(unittest.TestCase): "capacity": 1073741824 } }, 'state': 'Committed', - 'output_name': 'Output for step test_run_mounts', + 'output_name': 'Output from step test_run_mounts', 'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz', 'output_path': '/var/spool/cwl', 'output_ttl': 0, @@ -819,7 +819,7 @@ class TestContainer(unittest.TestCase): "capacity": 1073741824 } }, 'state': 'Committed', - 'output_name': 'Output for step test_secrets', + 'output_name': 'Output from step test_secrets', 'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz', 'output_path': '/var/spool/cwl', 'output_ttl': 0, @@ -941,7 +941,7 @@ class TestContainer(unittest.TestCase): "capacity": 1073741824 } }, 'state': 'Committed', - 'output_name': 'Output for step test_run_True', + 'output_name': 'Output from step test_run_True', 'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz', 'output_path': '/var/spool/cwl', 'output_ttl': 0, @@ -1027,7 +1027,7 @@ class TestContainer(unittest.TestCase): "capacity": 1073741824 } }, 'state': 'Committed', - 'output_name': 'Output for step test_run_True', + 'output_name': 'Output from step test_run_True', 'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz', 'output_path': '/var/spool/cwl', 'output_ttl': 0, @@ -1137,7 +1137,7 @@ class TestContainer(unittest.TestCase): "capacity": 1073741824 } }, 'state': 'Committed', - 'output_name': 'Output for step test_run_True' + ("" if test_case == 0 else "_"+str(test_case+1)), + 'output_name': 'Output from step test_run_True' + ("" if test_case == 0 else "_"+str(test_case+1)), 'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz', 'output_path': '/var/spool/cwl', 'output_ttl': 0, @@ -1211,7 +1211,7 @@ class TestContainer(unittest.TestCase): "capacity": 1073741824 } }, 'state': 'Committed', - 'output_name': 'Output for step test_run_True', + 'output_name': 'Output from step test_run_True', 'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz', 'output_path': '/var/spool/cwl', 'output_ttl': 0, @@ -1234,7 +1234,7 @@ class TestContainer(unittest.TestCase): runtimeContext.match_local_docker = True container_request['container_image'] = '99999999999999999999999999999993+99' container_request['name'] = 'test_run_True_2' - container_request['output_name'] = 'Output for step test_run_True_2' + container_request['output_name'] = 'Output from step test_run_True_2' for j in arvtool.job({}, mock.MagicMock(), runtimeContext): j.run(runtimeContext) runner.api.container_requests().create.assert_called_with( @@ -1324,7 +1324,7 @@ class TestContainer(unittest.TestCase): "capacity": 1073741824 } }, 'state': 'Committed', - 'output_name': 'Output for step '+runtimeContext.name, + 'output_name': 'Output from step '+runtimeContext.name, 'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz', 'output_path': '/var/spool/cwl', 'output_ttl': 0, @@ -1338,6 +1338,57 @@ class TestContainer(unittest.TestCase): })) + @mock.patch("arvados.commands.keepdocker.list_images_in_arv") + def test_output_properties(self, keepdocker): + arvados_cwl.add_arv_hints() + for rev in ["20210628", "20220510"]: + runner = mock.MagicMock() + runner.ignore_docker_for_reuse = False + runner.intermediate_output_ttl = 0 + runner.secret_store = cwltool.secrets.SecretStore() + runner.api._rootDesc = {"revision": rev} + + keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")] + runner.api.collections().get().execute.return_value = { + "portable_data_hash": "99999999999999999999999999999993+99"} + + tool = cmap({ + "inputs": [{ + "id": "inp", + "type": "string" + }], + "outputs": [], + "baseCommand": "ls", + "arguments": [{"valueFrom": "$(runtime.outdir)"}], + "id": "", + "cwlVersion": "v1.2", + "class": "CommandLineTool", + "hints": [ + { + "class": "http://arvados.org/cwl#OutputCollectionProperties", + "outputProperties": { + "foo": "bar", + "baz": "$(inputs.inp)" + } + } + ] + }) + + loadingContext, runtimeContext = self.helper(runner) + runtimeContext.name = "test_timelimit" + + arvtool = cwltool.load_tool.load_tool(tool, loadingContext) + arvtool.formatgraph = None + + for j in arvtool.job({"inp": "quux"}, mock.MagicMock(), runtimeContext): + j.run(runtimeContext) + + _, kwargs = runner.api.container_requests().create.call_args + if rev == "20220510": + self.assertEqual({"foo": "bar", "baz": "quux"}, kwargs['body'].get('output_properties')) + else: + self.assertEqual(None, kwargs['body'].get('output_properties')) + class TestWorkflow(unittest.TestCase): def setUp(self): @@ -1466,7 +1517,7 @@ class TestWorkflow(unittest.TestCase): } }, "name": "scatterstep", - "output_name": "Output for step scatterstep", + "output_name": "Output from step scatterstep", "output_path": "/var/spool/cwl", "output_ttl": 0, "priority": 500, @@ -1580,7 +1631,7 @@ class TestWorkflow(unittest.TestCase): u'cwl.input.yml' ], 'use_existing': True, - 'output_name': u'Output for step echo-subwf', + 'output_name': u'Output from step echo-subwf', 'cwd': '/var/spool/cwl', 'output_storage_classes': ["default"] })) diff --git a/sdk/cwl/tests/test_make_output.py b/sdk/cwl/tests/test_make_output.py index fe269592cb..dd1da0b524 100644 --- a/sdk/cwl/tests/test_make_output.py +++ b/sdk/cwl/tests/test_make_output.py @@ -50,7 +50,7 @@ class TestMakeOutput(unittest.TestCase): final.open.return_value = openmock openmock.__enter__.return_value = cwlout - _, runner.final_output_collection = runner.make_output_collection("Test output", ["foo"], "tag0,tag1,tag2", { + _, runner.final_output_collection = runner.make_output_collection("Test output", ["foo"], "tag0,tag1,tag2", {}, { "foo": { "class": "File", "location": "keep:99999999999999999999999999999991+99/foo.txt", @@ -67,7 +67,7 @@ class TestMakeOutput(unittest.TestCase): final.copy.assert_has_calls([mock.call('bar.txt', 'baz.txt', overwrite=False, source_collection=readermock)]) final.copy.assert_has_calls([mock.call('foo.txt', 'foo.txt', overwrite=False, source_collection=readermock)]) - final.save_new.assert_has_calls([mock.call(ensure_unique_name=True, name='Test output', owner_uuid='zzzzz-j7d0g-zzzzzzzzzzzzzzz', storage_classes=['foo'])]) + final.save_new.assert_has_calls([mock.call(ensure_unique_name=True, name='Test output', owner_uuid='zzzzz-j7d0g-zzzzzzzzzzzzzzz', properties={}, storage_classes=['foo'])]) self.assertEqual("""{ "bar": { "basename": "baz.txt", @@ -102,7 +102,7 @@ class TestMakeOutput(unittest.TestCase): reader.return_value = readermock # This output describes a single file listed in 2 different directories - _, runner.final_output_collection = runner.make_output_collection("Test output", ["foo"], "", { 'out': [ + _, runner.final_output_collection = runner.make_output_collection("Test output", ["foo"], "", {}, { 'out': [ { 'basename': 'testdir1', 'listing': [ @@ -152,7 +152,7 @@ class TestMakeOutput(unittest.TestCase): reader.return_value = readermock # This output describes two literals with the same basename - _, runner.final_output_collection = runner.make_output_collection("Test output", ["foo"], "", [ + _, runner.final_output_collection = runner.make_output_collection("Test output", ["foo"], "", {}, [ { 'lit': { diff --git a/sdk/cwl/tests/test_set_output_prop.py b/sdk/cwl/tests/test_set_output_prop.py new file mode 100644 index 0000000000..3219eac989 --- /dev/null +++ b/sdk/cwl/tests/test_set_output_prop.py @@ -0,0 +1,37 @@ +# Copyright (C) The Arvados Authors. All rights reserved. +# +# SPDX-License-Identifier: Apache-2.0 + +import arvados +import subprocess + +api = arvados.api() + +def test_execute(): + group = api.groups().create(body={"group": {"name": "test-17004-project", "group_class": "project"}}, ensure_unique_name=True).execute() + try: + contents = api.groups().contents(uuid=group["uuid"]).execute() + if len(contents["items"]) != 0: + raise Exception("Expected 0 items") + + cmd = ["arvados-cwl-runner", "--project-uuid", group["uuid"], "17004-output-props.cwl", "--inp", "scripts/download_all_data.sh"] + print(" ".join(cmd)) + subprocess.check_output(cmd) + + contents = api.groups().contents(uuid=group["uuid"]).execute() + + found = False + for c in contents["items"]: + if (c["kind"] == "arvados#collection" and + c["properties"].get("type") == "output" and + c["properties"].get("foo") == "bar" and + c["properties"].get("baz") == "download_all_data.sh"): + found = True + if not found: + raise Exception("Didn't find collection with properties") + + finally: + api.groups().delete(uuid=group["uuid"]).execute() + +if __name__ == '__main__': + test_execute() diff --git a/sdk/cwl/tests/test_submit.py b/sdk/cwl/tests/test_submit.py index 305d51e144..a726ec5017 100644 --- a/sdk/cwl/tests/test_submit.py +++ b/sdk/cwl/tests/test_submit.py @@ -45,311 +45,315 @@ import ruamel.yaml as yaml _rootDesc = None -def stubs(func): - @functools.wraps(func) - @mock.patch("arvados_cwl.arvdocker.determine_image_id") - @mock.patch("uuid.uuid4") - @mock.patch("arvados.commands.keepdocker.list_images_in_arv") - @mock.patch("arvados.collection.KeepClient") - @mock.patch("arvados.keep.KeepClient") - @mock.patch("arvados.events.subscribe") - def wrapped(self, events, keep_client1, keep_client2, keepdocker, - uuid4, determine_image_id, *args, **kwargs): - class Stubs(object): - pass - stubs = Stubs() - stubs.events = events - stubs.keepdocker = keepdocker - - uuid4.side_effect = ["df80736f-f14d-4b10-b2e3-03aa27f034bb", "df80736f-f14d-4b10-b2e3-03aa27f034b1", - "df80736f-f14d-4b10-b2e3-03aa27f034b2", "df80736f-f14d-4b10-b2e3-03aa27f034b3", - "df80736f-f14d-4b10-b2e3-03aa27f034b4", "df80736f-f14d-4b10-b2e3-03aa27f034b5"] - - determine_image_id.return_value = None - - def putstub(p, **kwargs): - return "%s+%i" % (hashlib.md5(p).hexdigest(), len(p)) - keep_client1().put.side_effect = putstub - keep_client1.put.side_effect = putstub - keep_client2().put.side_effect = putstub - keep_client2.put.side_effect = putstub - - stubs.keep_client = keep_client2 - stubs.docker_images = { - "arvados/jobs:"+arvados_cwl.__version__: [("zzzzz-4zz18-zzzzzzzzzzzzzd3", {})], - "debian:buster-slim": [("zzzzz-4zz18-zzzzzzzzzzzzzd4", {})], - "arvados/jobs:123": [("zzzzz-4zz18-zzzzzzzzzzzzzd5", {})], - "arvados/jobs:latest": [("zzzzz-4zz18-zzzzzzzzzzzzzd6", {})], - } - def kd(a, b, image_name=None, image_tag=None, project_uuid=None): - return stubs.docker_images.get("%s:%s" % (image_name, image_tag), []) - stubs.keepdocker.side_effect = kd +def stubs(wfname='submit_wf.cwl'): + def outer_wrapper(func, *rest): + @functools.wraps(func) + @mock.patch("arvados_cwl.arvdocker.determine_image_id") + @mock.patch("uuid.uuid4") + @mock.patch("arvados.commands.keepdocker.list_images_in_arv") + @mock.patch("arvados.collection.KeepClient") + @mock.patch("arvados.keep.KeepClient") + @mock.patch("arvados.events.subscribe") + def wrapped(self, events, keep_client1, keep_client2, keepdocker, + uuid4, determine_image_id, *args, **kwargs): + class Stubs(object): + pass + stubs = Stubs() + stubs.events = events + stubs.keepdocker = keepdocker + + uuid4.side_effect = ["df80736f-f14d-4b10-b2e3-03aa27f034bb", "df80736f-f14d-4b10-b2e3-03aa27f034b1", + "df80736f-f14d-4b10-b2e3-03aa27f034b2", "df80736f-f14d-4b10-b2e3-03aa27f034b3", + "df80736f-f14d-4b10-b2e3-03aa27f034b4", "df80736f-f14d-4b10-b2e3-03aa27f034b5"] + + determine_image_id.return_value = None + + def putstub(p, **kwargs): + return "%s+%i" % (hashlib.md5(p).hexdigest(), len(p)) + keep_client1().put.side_effect = putstub + keep_client1.put.side_effect = putstub + keep_client2().put.side_effect = putstub + keep_client2.put.side_effect = putstub + + stubs.keep_client = keep_client2 + stubs.docker_images = { + "arvados/jobs:"+arvados_cwl.__version__: [("zzzzz-4zz18-zzzzzzzzzzzzzd3", {})], + "debian:buster-slim": [("zzzzz-4zz18-zzzzzzzzzzzzzd4", {})], + "arvados/jobs:123": [("zzzzz-4zz18-zzzzzzzzzzzzzd5", {})], + "arvados/jobs:latest": [("zzzzz-4zz18-zzzzzzzzzzzzzd6", {})], + } + def kd(a, b, image_name=None, image_tag=None, project_uuid=None): + return stubs.docker_images.get("%s:%s" % (image_name, image_tag), []) + stubs.keepdocker.side_effect = kd - stubs.fake_user_uuid = "zzzzz-tpzed-zzzzzzzzzzzzzzz" - stubs.fake_container_uuid = "zzzzz-dz642-zzzzzzzzzzzzzzz" + stubs.fake_user_uuid = "zzzzz-tpzed-zzzzzzzzzzzzzzz" + stubs.fake_container_uuid = "zzzzz-dz642-zzzzzzzzzzzzzzz" - if sys.version_info[0] < 3: - stubs.capture_stdout = BytesIO() - else: - stubs.capture_stdout = StringIO() + if sys.version_info[0] < 3: + stubs.capture_stdout = BytesIO() + else: + stubs.capture_stdout = StringIO() - stubs.api = mock.MagicMock() - stubs.api._rootDesc = get_rootDesc() - stubs.api._rootDesc["uuidPrefix"] = "zzzzz" - stubs.api._rootDesc["revision"] = "20210628" + stubs.api = mock.MagicMock() + stubs.api._rootDesc = get_rootDesc() + stubs.api._rootDesc["uuidPrefix"] = "zzzzz" + stubs.api._rootDesc["revision"] = "20210628" - stubs.api.users().current().execute.return_value = { - "uuid": stubs.fake_user_uuid, - } - stubs.api.collections().list().execute.return_value = {"items": []} - stubs.api.containers().current().execute.return_value = { - "uuid": stubs.fake_container_uuid, - } - stubs.api.config()["StorageClasses"].items.return_value = { - "default": { - "Default": True - } - }.items() - - class CollectionExecute(object): - def __init__(self, exe): - self.exe = exe - def execute(self, num_retries=None): - return self.exe - - def collection_createstub(created_collections, body, ensure_unique_name=None): - mt = body["manifest_text"].encode('utf-8') - uuid = "zzzzz-4zz18-zzzzzzzzzzzzzx%d" % len(created_collections) - pdh = "%s+%i" % (hashlib.md5(mt).hexdigest(), len(mt)) - created_collections[uuid] = { - "uuid": uuid, - "portable_data_hash": pdh, - "manifest_text": mt.decode('utf-8') + stubs.api.users().current().execute.return_value = { + "uuid": stubs.fake_user_uuid, } - return CollectionExecute(created_collections[uuid]) - - def collection_getstub(created_collections, uuid): - for v in viewvalues(created_collections): - if uuid in (v["uuid"], v["portable_data_hash"]): - return CollectionExecute(v) - - created_collections = { - "99999999999999999999999999999998+99": { - "uuid": "", - "portable_data_hash": "99999999999999999999999999999998+99", - "manifest_text": ". 99999999999999999999999999999998+99 0:0:file1.txt" - }, - "99999999999999999999999999999997+99": { - "uuid": "", - "portable_data_hash": "99999999999999999999999999999997+99", - "manifest_text": ". 99999999999999999999999999999997+99 0:0:file1.txt" - }, - "99999999999999999999999999999994+99": { - "uuid": "", - "portable_data_hash": "99999999999999999999999999999994+99", - "manifest_text": ". 99999999999999999999999999999994+99 0:0:expect_arvworkflow.cwl" - }, - "zzzzz-4zz18-zzzzzzzzzzzzzd3": { - "uuid": "zzzzz-4zz18-zzzzzzzzzzzzzd3", - "portable_data_hash": "999999999999999999999999999999d3+99", - "manifest_text": "" - }, - "zzzzz-4zz18-zzzzzzzzzzzzzd4": { - "uuid": "zzzzz-4zz18-zzzzzzzzzzzzzd4", - "portable_data_hash": "999999999999999999999999999999d4+99", - "manifest_text": "" - }, - "zzzzz-4zz18-zzzzzzzzzzzzzd5": { - "uuid": "zzzzz-4zz18-zzzzzzzzzzzzzd5", - "portable_data_hash": "999999999999999999999999999999d5+99", - "manifest_text": "" - }, - "zzzzz-4zz18-zzzzzzzzzzzzzd6": { - "uuid": "zzzzz-4zz18-zzzzzzzzzzzzzd6", - "portable_data_hash": "999999999999999999999999999999d6+99", - "manifest_text": "" + stubs.api.collections().list().execute.return_value = {"items": []} + stubs.api.containers().current().execute.return_value = { + "uuid": stubs.fake_container_uuid, } - } - stubs.api.collections().create.side_effect = functools.partial(collection_createstub, created_collections) - stubs.api.collections().get.side_effect = functools.partial(collection_getstub, created_collections) - - stubs.expect_job_uuid = "zzzzz-8i9sb-zzzzzzzzzzzzzzz" - stubs.api.jobs().create().execute.return_value = { - "uuid": stubs.expect_job_uuid, - "state": "Queued", - } - - stubs.expect_container_request_uuid = "zzzzz-xvhdp-zzzzzzzzzzzzzzz" - stubs.api.container_requests().create().execute.return_value = { - "uuid": stubs.expect_container_request_uuid, - "container_uuid": "zzzzz-dz642-zzzzzzzzzzzzzzz", - "state": "Queued" - } - - stubs.expect_pipeline_template_uuid = "zzzzz-d1hrv-zzzzzzzzzzzzzzz" - stubs.api.pipeline_templates().create().execute.return_value = { - "uuid": stubs.expect_pipeline_template_uuid, - } - stubs.expect_job_spec = { - 'runtime_constraints': { - 'docker_image': '999999999999999999999999999999d3+99', - 'min_ram_mb_per_node': 1024 - }, - 'script_parameters': { - 'x': { - 'basename': 'blorp.txt', - 'location': 'keep:169f39d466a5438ac4a90e779bf750c7+53/blorp.txt', - 'class': 'File' + stubs.api.config()["StorageClasses"].items.return_value = { + "default": { + "Default": True + } + }.items() + + class CollectionExecute(object): + def __init__(self, exe): + self.exe = exe + def execute(self, num_retries=None): + return self.exe + + def collection_createstub(created_collections, body, ensure_unique_name=None): + mt = body["manifest_text"].encode('utf-8') + uuid = "zzzzz-4zz18-zzzzzzzzzzzzzx%d" % len(created_collections) + pdh = "%s+%i" % (hashlib.md5(mt).hexdigest(), len(mt)) + created_collections[uuid] = { + "uuid": uuid, + "portable_data_hash": pdh, + "manifest_text": mt.decode('utf-8') + } + return CollectionExecute(created_collections[uuid]) + + def collection_getstub(created_collections, uuid): + for v in viewvalues(created_collections): + if uuid in (v["uuid"], v["portable_data_hash"]): + return CollectionExecute(v) + + created_collections = { + "99999999999999999999999999999998+99": { + "uuid": "", + "portable_data_hash": "99999999999999999999999999999998+99", + "manifest_text": ". 99999999999999999999999999999998+99 0:0:file1.txt" }, - 'y': { - 'basename': '99999999999999999999999999999998+99', - 'location': 'keep:99999999999999999999999999999998+99', - 'class': 'Directory' + "99999999999999999999999999999997+99": { + "uuid": "", + "portable_data_hash": "99999999999999999999999999999997+99", + "manifest_text": ". 99999999999999999999999999999997+99 0:0:file1.txt" }, - 'z': { - 'basename': 'anonymous', - "listing": [{ - "basename": "renamed.txt", - "class": "File", - "location": "keep:99999999999999999999999999999998+99/file1.txt", - "size": 0 - }], - 'class': 'Directory' + "99999999999999999999999999999994+99": { + "uuid": "", + "portable_data_hash": "99999999999999999999999999999994+99", + "manifest_text": ". 99999999999999999999999999999994+99 0:0:expect_arvworkflow.cwl" }, - 'cwl:tool': '57ad063d64c60dbddc027791f0649211+60/workflow.cwl#main' - }, - 'repository': 'arvados', - 'script_version': 'master', - 'minimum_script_version': '570509ab4d2ef93d870fd2b1f2eab178afb1bad9', - 'script': 'cwl-runner' - } - stubs.pipeline_component = stubs.expect_job_spec.copy() - stubs.expect_pipeline_instance = { - 'name': 'submit_wf.cwl', - 'state': 'RunningOnServer', - 'owner_uuid': None, - "components": { - "cwl-runner": { - 'runtime_constraints': {'docker_image': '999999999999999999999999999999d3+99', 'min_ram_mb_per_node': 1024}, - 'script_parameters': { - 'y': {"value": {'basename': '99999999999999999999999999999998+99', 'location': 'keep:99999999999999999999999999999998+99', 'class': 'Directory'}}, - 'x': {"value": { - 'basename': 'blorp.txt', - 'class': 'File', - 'location': 'keep:169f39d466a5438ac4a90e779bf750c7+53/blorp.txt', - "size": 16 - }}, - 'z': {"value": {'basename': 'anonymous', 'class': 'Directory', - 'listing': [ - { - 'basename': 'renamed.txt', - 'class': 'File', 'location': - 'keep:99999999999999999999999999999998+99/file1.txt', - 'size': 0 - } - ]}}, - 'cwl:tool': '57ad063d64c60dbddc027791f0649211+60/workflow.cwl#main', - 'arv:debug': True, - 'arv:enable_reuse': True, - 'arv:on_error': 'continue' - }, - 'repository': 'arvados', - 'script_version': 'master', - 'minimum_script_version': '570509ab4d2ef93d870fd2b1f2eab178afb1bad9', - 'script': 'cwl-runner', - 'job': {'state': 'Queued', 'uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz'} + "zzzzz-4zz18-zzzzzzzzzzzzzd3": { + "uuid": "zzzzz-4zz18-zzzzzzzzzzzzzd3", + "portable_data_hash": "999999999999999999999999999999d3+99", + "manifest_text": "" + }, + "zzzzz-4zz18-zzzzzzzzzzzzzd4": { + "uuid": "zzzzz-4zz18-zzzzzzzzzzzzzd4", + "portable_data_hash": "999999999999999999999999999999d4+99", + "manifest_text": "" + }, + "zzzzz-4zz18-zzzzzzzzzzzzzd5": { + "uuid": "zzzzz-4zz18-zzzzzzzzzzzzzd5", + "portable_data_hash": "999999999999999999999999999999d5+99", + "manifest_text": "" + }, + "zzzzz-4zz18-zzzzzzzzzzzzzd6": { + "uuid": "zzzzz-4zz18-zzzzzzzzzzzzzd6", + "portable_data_hash": "999999999999999999999999999999d6+99", + "manifest_text": "" } } - } - stubs.pipeline_create = copy.deepcopy(stubs.expect_pipeline_instance) - stubs.expect_pipeline_uuid = "zzzzz-d1hrv-zzzzzzzzzzzzzzz" - stubs.pipeline_create["uuid"] = stubs.expect_pipeline_uuid - stubs.pipeline_with_job = copy.deepcopy(stubs.pipeline_create) - stubs.pipeline_with_job["components"]["cwl-runner"]["job"] = { - "uuid": "zzzzz-8i9sb-zzzzzzzzzzzzzzz", - "state": "Queued" - } - stubs.api.pipeline_instances().create().execute.return_value = stubs.pipeline_create - stubs.api.pipeline_instances().get().execute.return_value = stubs.pipeline_with_job + stubs.api.collections().create.side_effect = functools.partial(collection_createstub, created_collections) + stubs.api.collections().get.side_effect = functools.partial(collection_getstub, created_collections) - with open("tests/wf/submit_wf_packed.cwl") as f: - expect_packed_workflow = yaml.round_trip_load(f) + stubs.expect_job_uuid = "zzzzz-8i9sb-zzzzzzzzzzzzzzz" + stubs.api.jobs().create().execute.return_value = { + "uuid": stubs.expect_job_uuid, + "state": "Queued", + } - stubs.expect_container_spec = { - 'priority': 500, - 'mounts': { - '/var/spool/cwl': { - 'writable': True, - 'kind': 'collection' - }, - '/var/lib/cwl/workflow.json': { - 'content': expect_packed_workflow, - 'kind': 'json' + stubs.expect_container_request_uuid = "zzzzz-xvhdp-zzzzzzzzzzzzzzz" + stubs.api.container_requests().create().execute.return_value = { + "uuid": stubs.expect_container_request_uuid, + "container_uuid": "zzzzz-dz642-zzzzzzzzzzzzzzz", + "state": "Queued" + } + + stubs.expect_pipeline_template_uuid = "zzzzz-d1hrv-zzzzzzzzzzzzzzz" + stubs.api.pipeline_templates().create().execute.return_value = { + "uuid": stubs.expect_pipeline_template_uuid, + } + stubs.expect_job_spec = { + 'runtime_constraints': { + 'docker_image': '999999999999999999999999999999d3+99', + 'min_ram_mb_per_node': 1024 }, - 'stdout': { - 'path': '/var/spool/cwl/cwl.output.json', - 'kind': 'file' + 'script_parameters': { + 'x': { + 'basename': 'blorp.txt', + 'location': 'keep:169f39d466a5438ac4a90e779bf750c7+53/blorp.txt', + 'class': 'File' + }, + 'y': { + 'basename': '99999999999999999999999999999998+99', + 'location': 'keep:99999999999999999999999999999998+99', + 'class': 'Directory' + }, + 'z': { + 'basename': 'anonymous', + "listing": [{ + "basename": "renamed.txt", + "class": "File", + "location": "keep:99999999999999999999999999999998+99/file1.txt", + "size": 0 + }], + 'class': 'Directory' + }, + 'cwl:tool': '57ad063d64c60dbddc027791f0649211+60/workflow.cwl#main' }, - '/var/lib/cwl/cwl.input.json': { - 'kind': 'json', - 'content': { - 'y': { - 'basename': '99999999999999999999999999999998+99', - 'location': 'keep:99999999999999999999999999999998+99', - 'class': 'Directory'}, - 'x': { - 'basename': u'blorp.txt', - 'class': 'File', - 'location': u'keep:169f39d466a5438ac4a90e779bf750c7+53/blorp.txt', - "size": 16 + 'repository': 'arvados', + 'script_version': 'master', + 'minimum_script_version': '570509ab4d2ef93d870fd2b1f2eab178afb1bad9', + 'script': 'cwl-runner' + } + stubs.pipeline_component = stubs.expect_job_spec.copy() + stubs.expect_pipeline_instance = { + 'name': 'submit_wf.cwl', + 'state': 'RunningOnServer', + 'owner_uuid': None, + "components": { + "cwl-runner": { + 'runtime_constraints': {'docker_image': '999999999999999999999999999999d3+99', 'min_ram_mb_per_node': 1024}, + 'script_parameters': { + 'y': {"value": {'basename': '99999999999999999999999999999998+99', 'location': 'keep:99999999999999999999999999999998+99', 'class': 'Directory'}}, + 'x': {"value": { + 'basename': 'blorp.txt', + 'class': 'File', + 'location': 'keep:169f39d466a5438ac4a90e779bf750c7+53/blorp.txt', + "size": 16 + }}, + 'z': {"value": {'basename': 'anonymous', 'class': 'Directory', + 'listing': [ + { + 'basename': 'renamed.txt', + 'class': 'File', 'location': + 'keep:99999999999999999999999999999998+99/file1.txt', + 'size': 0 + } + ]}}, + 'cwl:tool': '57ad063d64c60dbddc027791f0649211+60/workflow.cwl#main', + 'arv:debug': True, + 'arv:enable_reuse': True, + 'arv:on_error': 'continue' }, - 'z': {'basename': 'anonymous', 'class': 'Directory', 'listing': [ - {'basename': 'renamed.txt', - 'class': 'File', - 'location': 'keep:99999999999999999999999999999998+99/file1.txt', - 'size': 0 - } - ]} - }, - 'kind': 'json' + 'repository': 'arvados', + 'script_version': 'master', + 'minimum_script_version': '570509ab4d2ef93d870fd2b1f2eab178afb1bad9', + 'script': 'cwl-runner', + 'job': {'state': 'Queued', 'uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz'} + } } - }, - 'secret_mounts': {}, - 'state': 'Committed', - 'command': ['arvados-cwl-runner', '--local', '--api=containers', - '--no-log-timestamps', '--disable-validate', '--disable-color', - '--eval-timeout=20', '--thread-count=0', - '--enable-reuse', "--collection-cache-size=256", '--debug', '--on-error=continue', - '/var/lib/cwl/workflow.json#main', '/var/lib/cwl/cwl.input.json'], - 'name': 'submit_wf.cwl', - 'container_image': '999999999999999999999999999999d3+99', - 'output_path': '/var/spool/cwl', - 'cwd': '/var/spool/cwl', - 'runtime_constraints': { - 'API': True, - 'vcpus': 1, - 'ram': (1024+256)*1024*1024 - }, - 'use_existing': False, - 'properties': {}, - 'secret_mounts': {} - } - - stubs.expect_workflow_uuid = "zzzzz-7fd4e-zzzzzzzzzzzzzzz" - stubs.api.workflows().create().execute.return_value = { - "uuid": stubs.expect_workflow_uuid, - } - def update_mock(**kwargs): - stubs.updated_uuid = kwargs.get('uuid') - return mock.DEFAULT - stubs.api.workflows().update.side_effect = update_mock - stubs.api.workflows().update().execute.side_effect = lambda **kwargs: { - "uuid": stubs.updated_uuid, - } + } + stubs.pipeline_create = copy.deepcopy(stubs.expect_pipeline_instance) + stubs.expect_pipeline_uuid = "zzzzz-d1hrv-zzzzzzzzzzzzzzz" + stubs.pipeline_create["uuid"] = stubs.expect_pipeline_uuid + stubs.pipeline_with_job = copy.deepcopy(stubs.pipeline_create) + stubs.pipeline_with_job["components"]["cwl-runner"]["job"] = { + "uuid": "zzzzz-8i9sb-zzzzzzzzzzzzzzz", + "state": "Queued" + } + stubs.api.pipeline_instances().create().execute.return_value = stubs.pipeline_create + stubs.api.pipeline_instances().get().execute.return_value = stubs.pipeline_with_job + + with open("tests/wf/submit_wf_packed.cwl") as f: + expect_packed_workflow = yaml.round_trip_load(f) + + stubs.expect_container_spec = { + 'priority': 500, + 'mounts': { + '/var/spool/cwl': { + 'writable': True, + 'kind': 'collection' + }, + '/var/lib/cwl/workflow.json': { + 'content': expect_packed_workflow, + 'kind': 'json' + }, + 'stdout': { + 'path': '/var/spool/cwl/cwl.output.json', + 'kind': 'file' + }, + '/var/lib/cwl/cwl.input.json': { + 'kind': 'json', + 'content': { + 'y': { + 'basename': '99999999999999999999999999999998+99', + 'location': 'keep:99999999999999999999999999999998+99', + 'class': 'Directory'}, + 'x': { + 'basename': u'blorp.txt', + 'class': 'File', + 'location': u'keep:169f39d466a5438ac4a90e779bf750c7+53/blorp.txt', + "size": 16 + }, + 'z': {'basename': 'anonymous', 'class': 'Directory', 'listing': [ + {'basename': 'renamed.txt', + 'class': 'File', + 'location': 'keep:99999999999999999999999999999998+99/file1.txt', + 'size': 0 + } + ]} + }, + 'kind': 'json' + } + }, + 'secret_mounts': {}, + 'state': 'Committed', + 'command': ['arvados-cwl-runner', '--local', '--api=containers', + '--no-log-timestamps', '--disable-validate', '--disable-color', + '--eval-timeout=20', '--thread-count=0', + '--enable-reuse', "--collection-cache-size=256", + '--output-name=Output from workflow '+wfname, + '--debug', '--on-error=continue', + '/var/lib/cwl/workflow.json#main', '/var/lib/cwl/cwl.input.json'], + 'name': wfname, + 'container_image': '999999999999999999999999999999d3+99', + 'output_name': 'Output from workflow '+wfname, + 'output_path': '/var/spool/cwl', + 'cwd': '/var/spool/cwl', + 'runtime_constraints': { + 'API': True, + 'vcpus': 1, + 'ram': (1024+256)*1024*1024 + }, + 'use_existing': False, + 'properties': {}, + 'secret_mounts': {} + } - return func(self, stubs, *args, **kwargs) - return wrapped + stubs.expect_workflow_uuid = "zzzzz-7fd4e-zzzzzzzzzzzzzzz" + stubs.api.workflows().create().execute.return_value = { + "uuid": stubs.expect_workflow_uuid, + } + def update_mock(**kwargs): + stubs.updated_uuid = kwargs.get('uuid') + return mock.DEFAULT + stubs.api.workflows().update.side_effect = update_mock + stubs.api.workflows().update().execute.side_effect = lambda **kwargs: { + "uuid": stubs.updated_uuid, + } + return func(self, stubs, *args, **kwargs) + return wrapped + return outer_wrapper class TestSubmit(unittest.TestCase): @@ -429,6 +433,7 @@ class TestSubmit(unittest.TestCase): '--no-log-timestamps', '--disable-validate', '--disable-color', '--eval-timeout=20', '--thread-count=0', '--disable-reuse', "--collection-cache-size=256", + "--output-name=Output from workflow submit_wf.cwl", '--debug', '--on-error=continue', '/var/lib/cwl/workflow.json#main', '/var/lib/cwl/cwl.input.json'] expect_container["use_existing"] = False @@ -439,7 +444,7 @@ class TestSubmit(unittest.TestCase): stubs.expect_container_request_uuid + '\n') self.assertEqual(exited, 0) - @stubs + @stubs('submit_wf_no_reuse.cwl') def test_submit_container_reuse_disabled_by_workflow(self, stubs): exited = arvados_cwl.main( ["--submit", "--no-wait", "--api=containers", "--debug", @@ -452,10 +457,10 @@ class TestSubmit(unittest.TestCase): 'arvados-cwl-runner', '--local', '--api=containers', '--no-log-timestamps', '--disable-validate', '--disable-color', '--eval-timeout=20', '--thread-count=0', - '--disable-reuse', "--collection-cache-size=256", '--debug', '--on-error=continue', + '--disable-reuse', "--collection-cache-size=256", + '--output-name=Output from workflow submit_wf_no_reuse.cwl', '--debug', '--on-error=continue', '/var/lib/cwl/workflow.json#main', '/var/lib/cwl/cwl.input.json'] expect_container["use_existing"] = False - expect_container["name"] = "submit_wf_no_reuse.cwl" expect_container["mounts"]["/var/lib/cwl/workflow.json"]["content"]["$graph"][1]["hints"] = [ { "class": "http://arvados.org/cwl#ReuseRequirement", @@ -485,6 +490,7 @@ class TestSubmit(unittest.TestCase): '--no-log-timestamps', '--disable-validate', '--disable-color', '--eval-timeout=20', '--thread-count=0', '--enable-reuse', "--collection-cache-size=256", + "--output-name=Output from workflow submit_wf.cwl", '--debug', '--on-error=stop', '/var/lib/cwl/workflow.json#main', '/var/lib/cwl/cwl.input.json'] @@ -529,7 +535,9 @@ class TestSubmit(unittest.TestCase): expect_container["command"] = ['arvados-cwl-runner', '--local', '--api=containers', '--no-log-timestamps', '--disable-validate', '--disable-color', '--eval-timeout=20', '--thread-count=0', - '--enable-reuse', "--collection-cache-size=256", "--debug", + '--enable-reuse', "--collection-cache-size=256", + '--output-name=Output from workflow submit_wf.cwl', + "--debug", "--storage-classes=foo", '--on-error=continue', '/var/lib/cwl/workflow.json#main', '/var/lib/cwl/cwl.input.json'] @@ -550,7 +558,9 @@ class TestSubmit(unittest.TestCase): expect_container["command"] = ['arvados-cwl-runner', '--local', '--api=containers', '--no-log-timestamps', '--disable-validate', '--disable-color', '--eval-timeout=20', '--thread-count=0', - '--enable-reuse', "--collection-cache-size=256", "--debug", + '--enable-reuse', "--collection-cache-size=256", + "--output-name=Output from workflow submit_wf.cwl", + "--debug", "--storage-classes=foo,bar", "--intermediate-storage-classes=baz", '--on-error=continue', '/var/lib/cwl/workflow.json#main', '/var/lib/cwl/cwl.input.json'] @@ -569,7 +579,7 @@ class TestSubmit(unittest.TestCase): make_output.return_value = ({},final_output_c) def set_final_output(job_order, output_callback, runtimeContext): - output_callback("zzzzz-4zz18-zzzzzzzzzzzzzzzz", "success") + output_callback({"out": "zzzzz"}, "success") return [] job.side_effect = set_final_output @@ -578,7 +588,7 @@ class TestSubmit(unittest.TestCase): "tests/wf/submit_wf.cwl", "tests/submit_test_job.json"], stubs.capture_stdout, sys.stderr, api_client=stubs.api, keep_client=stubs.keep_client) - make_output.assert_called_with(u'Output of submit_wf.cwl', ['foo'], '', 'zzzzz-4zz18-zzzzzzzzzzzzzzzz') + make_output.assert_called_with(u'Output of submit_wf.cwl', ['foo'], '', {}, {"out": "zzzzz"}) self.assertEqual(exited, 0) @mock.patch("cwltool.task_queue.TaskQueue") @@ -591,7 +601,7 @@ class TestSubmit(unittest.TestCase): stubs.api.config().get.return_value = {"default": {"Default": True}} def set_final_output(job_order, output_callback, runtimeContext): - output_callback("zzzzz-4zz18-zzzzzzzzzzzzzzzz", "success") + output_callback({"out": "zzzzz"}, "success") return [] job.side_effect = set_final_output @@ -600,7 +610,7 @@ class TestSubmit(unittest.TestCase): "tests/wf/submit_wf.cwl", "tests/submit_test_job.json"], stubs.capture_stdout, sys.stderr, api_client=stubs.api, keep_client=stubs.keep_client) - make_output.assert_called_with(u'Output of submit_wf.cwl', ['default'], '', 'zzzzz-4zz18-zzzzzzzzzzzzzzzz') + make_output.assert_called_with(u'Output of submit_wf.cwl', ['default'], '', {}, {"out": "zzzzz"}) self.assertEqual(exited, 0) @mock.patch("cwltool.task_queue.TaskQueue") @@ -612,7 +622,7 @@ class TestSubmit(unittest.TestCase): make_output.return_value = ({},final_output_c) def set_final_output(job_order, output_callback, runtimeContext): - output_callback("zzzzz-4zz18-zzzzzzzzzzzzzzzz", "success") + output_callback({"out": "zzzzz"}, "success") return [] job.side_effect = set_final_output @@ -621,7 +631,7 @@ class TestSubmit(unittest.TestCase): "tests/wf/submit_storage_class_wf.cwl", "tests/submit_test_job.json"], stubs.capture_stdout, sys.stderr, api_client=stubs.api, keep_client=stubs.keep_client) - make_output.assert_called_with(u'Output of submit_storage_class_wf.cwl', ['foo', 'bar'], '', 'zzzzz-4zz18-zzzzzzzzzzzzzzzz') + make_output.assert_called_with(u'Output of submit_storage_class_wf.cwl', ['foo', 'bar'], '', {}, {"out": "zzzzz"}) self.assertEqual(exited, 0) @stubs @@ -635,7 +645,8 @@ class TestSubmit(unittest.TestCase): expect_container["command"] = ['arvados-cwl-runner', '--local', '--api=containers', '--no-log-timestamps', '--disable-validate', '--disable-color', '--eval-timeout=20', '--thread-count=0', - '--enable-reuse', "--collection-cache-size=256", '--debug', + '--enable-reuse', "--collection-cache-size=256", + "--output-name=Output from workflow submit_wf.cwl", '--debug', '--on-error=continue', "--intermediate-output-ttl=3600", '/var/lib/cwl/workflow.json#main', '/var/lib/cwl/cwl.input.json'] @@ -683,6 +694,7 @@ class TestSubmit(unittest.TestCase): '--no-log-timestamps', '--disable-validate', '--disable-color', '--eval-timeout=20', '--thread-count=0', '--enable-reuse', "--collection-cache-size=256", + "--output-name=Output from workflow submit_wf.cwl", "--output-tags="+output_tags, '--debug', '--on-error=continue', '/var/lib/cwl/workflow.json#main', '/var/lib/cwl/cwl.input.json'] @@ -756,11 +768,14 @@ class TestSubmit(unittest.TestCase): }, 'state': 'Committed', 'output_path': '/var/spool/cwl', 'name': 'expect_arvworkflow.cwl#main', + 'output_name': 'Output from workflow expect_arvworkflow.cwl#main', 'container_image': '999999999999999999999999999999d3+99', 'command': ['arvados-cwl-runner', '--local', '--api=containers', '--no-log-timestamps', '--disable-validate', '--disable-color', '--eval-timeout=20', '--thread-count=0', - '--enable-reuse', "--collection-cache-size=256", '--debug', '--on-error=continue', + '--enable-reuse', "--collection-cache-size=256", + '--output-name=Output from workflow expect_arvworkflow.cwl#main', + '--debug', '--on-error=continue', '/var/lib/cwl/workflow/expect_arvworkflow.cwl#main', '/var/lib/cwl/cwl.input.json'], 'cwd': '/var/spool/cwl', 'runtime_constraints': { @@ -876,7 +891,7 @@ class TestSubmit(unittest.TestCase): stubs.expect_container_request_uuid + '\n') self.assertEqual(exited, 0) - @stubs + @stubs('hello container 123') def test_submit_container_name(self, stubs): exited = arvados_cwl.main( ["--submit", "--no-wait", "--api=containers", "--debug", "--name=hello container 123", @@ -884,7 +899,6 @@ class TestSubmit(unittest.TestCase): stubs.capture_stdout, sys.stderr, api_client=stubs.api, keep_client=stubs.keep_client) expect_container = copy.deepcopy(stubs.expect_container_spec) - expect_container["name"] = "hello container 123" stubs.api.container_requests().create.assert_called_with( body=JsonDiffMatcher(expect_container)) @@ -920,7 +934,8 @@ class TestSubmit(unittest.TestCase): expect_container["command"] = ['arvados-cwl-runner', '--local', '--api=containers', '--no-log-timestamps', '--disable-validate', '--disable-color', "--eval-timeout=20", "--thread-count=0", - '--enable-reuse', "--collection-cache-size=256", '--debug', + '--enable-reuse', "--collection-cache-size=256", + "--output-name=Output from workflow submit_wf.cwl", '--debug', '--on-error=continue', '--project-uuid='+project_uuid, '/var/lib/cwl/workflow.json#main', '/var/lib/cwl/cwl.input.json'] @@ -1027,7 +1042,7 @@ class TestSubmit(unittest.TestCase): stubs.expect_container_request_uuid + '\n') self.assertEqual(exited, 0) - @stubs + @stubs('submit_wf_runner_resources.cwl') def test_submit_wf_runner_resources(self, stubs): exited = arvados_cwl.main( ["--submit", "--no-wait", "--api=containers", "--debug", @@ -1040,7 +1055,6 @@ class TestSubmit(unittest.TestCase): "vcpus": 2, "ram": (2000+512) * 2**20 } - expect_container["name"] = "submit_wf_runner_resources.cwl" expect_container["mounts"]["/var/lib/cwl/workflow.json"]["content"]["$graph"][1]["hints"] = [ { "class": "http://arvados.org/cwl#WorkflowRunnerResources", @@ -1055,7 +1069,9 @@ class TestSubmit(unittest.TestCase): expect_container['command'] = ['arvados-cwl-runner', '--local', '--api=containers', '--no-log-timestamps', '--disable-validate', '--disable-color', '--eval-timeout=20', '--thread-count=0', - '--enable-reuse', "--collection-cache-size=512", '--debug', '--on-error=continue', + '--enable-reuse', "--collection-cache-size=512", + '--output-name=Output from workflow submit_wf_runner_resources.cwl', + '--debug', '--on-error=continue', '/var/lib/cwl/workflow.json#main', '/var/lib/cwl/cwl.input.json'] stubs.api.container_requests().create.assert_called_with( @@ -1139,6 +1155,7 @@ class TestSubmit(unittest.TestCase): '--thread-count=0', "--enable-reuse", "--collection-cache-size=256", + '--output-name=Output from workflow secret_wf.cwl' '--debug', "--on-error=continue", "/var/lib/cwl/workflow.json#main", @@ -1264,6 +1281,7 @@ class TestSubmit(unittest.TestCase): } }, "name": "secret_wf.cwl", + "output_name": "Output from workflow secret_wf.cwl", "output_path": "/var/spool/cwl", "priority": 500, "properties": {}, @@ -1452,7 +1470,7 @@ class TestSubmit(unittest.TestCase): finally: cwltool_logger.removeHandler(stderr_logger) - @stubs + @stubs('submit_wf_process_properties.cwl') def test_submit_set_process_properties(self, stubs): exited = arvados_cwl.main( ["--submit", "--no-wait", "--api=containers", "--debug", @@ -1460,7 +1478,7 @@ class TestSubmit(unittest.TestCase): stubs.capture_stdout, sys.stderr, api_client=stubs.api, keep_client=stubs.keep_client) expect_container = copy.deepcopy(stubs.expect_container_spec) - expect_container["name"] = "submit_wf_process_properties.cwl" + expect_container["mounts"]["/var/lib/cwl/workflow.json"]["content"]["$graph"][1]["hints"] = [ { "class": "http://arvados.org/cwl#ProcessProperties", diff --git a/sdk/go/arvados/container.go b/sdk/go/arvados/container.go index 3510a6db04..de709980fd 100644 --- a/sdk/go/arvados/container.go +++ b/sdk/go/arvados/container.go @@ -75,6 +75,7 @@ type ContainerRequest struct { Filters []Filter `json:"filters"` ContainerCount int `json:"container_count"` OutputStorageClasses []string `json:"output_storage_classes"` + OutputProperties map[string]interface{} `json:"output_properties"` } // Mount is special behavior to attach to a filesystem path or device. diff --git a/sdk/python/arvados/api.py b/sdk/python/arvados/api.py index e0d1c50f03..db1d0f4e12 100644 --- a/sdk/python/arvados/api.py +++ b/sdk/python/arvados/api.py @@ -133,6 +133,10 @@ def _patch_http_request(http, api_token): http._request_id = util.new_request_id return http +def _close_connections(self): + for conn in self._http.connections.values(): + conn.close() + # Monkey patch discovery._cast() so objects and arrays get serialized # with json.dumps() instead of str(). _cast_orig = apiclient_discovery._cast @@ -254,6 +258,7 @@ def api(version=None, cache=True, host=None, token=None, insecure=False, svc.request_id = request_id svc.config = lambda: util.get_config_once(svc) svc.vocabulary = lambda: util.get_vocabulary_once(svc) + svc.close_connections = types.MethodType(_close_connections, svc) kwargs['http'].max_request_size = svc._rootDesc.get('maxRequestSize', 0) kwargs['http'].cache = None kwargs['http']._request_id = lambda: svc.request_id or util.new_request_id() diff --git a/sdk/python/arvados/commands/get.py b/sdk/python/arvados/commands/get.py index c4262c59c9..bb421def61 100755 --- a/sdk/python/arvados/commands/get.py +++ b/sdk/python/arvados/commands/get.py @@ -17,7 +17,6 @@ import arvados.util as util from arvados._version import __version__ -api_client = None logger = logging.getLogger('arvados.arv-get') parser = argparse.ArgumentParser( @@ -146,8 +145,6 @@ def parse_arguments(arguments, stdout, stderr): return args def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr): - global api_client - if stdout is sys.stdout and hasattr(stdout, 'buffer'): # in Python 3, write to stdout as binary stdout = stdout.buffer @@ -158,8 +155,7 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr): request_id = arvados.util.new_request_id() logger.info('X-Request-Id: '+request_id) - if api_client is None: - api_client = arvados.api('v1', request_id=request_id) + api_client = arvados.api('v1', request_id=request_id) r = re.search(r'^(.*?)(/.*)?$', args.locator) col_loc = r.group(1) diff --git a/sdk/python/tests/test_arv_get.py b/sdk/python/tests/test_arv_get.py index 733cd6478c..73ef2475b9 100644 --- a/sdk/python/tests/test_arv_get.py +++ b/sdk/python/tests/test_arv_get.py @@ -49,12 +49,15 @@ class ArvadosGetTestCase(run_test_server.TestCaseWithServers, 'bar.txt' : 'bar', 'subdir/baz.txt' : 'baz', }): - c = collection.Collection() + api = arvados.api() + c = collection.Collection(api_client=api) for path, data in listitems(contents): with c.open(path, 'wb') as f: f.write(data) c.save_new() + api.close_connections() + return (c.manifest_locator(), c.portable_data_hash(), c.manifest_text(strip=strip_manifest)) diff --git a/services/api/app/controllers/arvados/v1/schema_controller.rb b/services/api/app/controllers/arvados/v1/schema_controller.rb index 5508ac0fbd..0300b75075 100644 --- a/services/api/app/controllers/arvados/v1/schema_controller.rb +++ b/services/api/app/controllers/arvados/v1/schema_controller.rb @@ -37,7 +37,7 @@ class Arvados::V1::SchemaController < ApplicationController # format is YYYYMMDD, must be fixed width (needs to be lexically # sortable), updated manually, may be used by clients to # determine availability of API server features. - revision: "20220222", + revision: "20220510", source_version: AppVersion.hash, sourceVersion: AppVersion.hash, # source_version should be deprecated in the future packageVersion: AppVersion.package_version, diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb index 0326b12985..3a04c56046 100644 --- a/services/api/app/models/container.rb +++ b/services/api/app/models/container.rb @@ -23,6 +23,7 @@ class Container < ArvadosModel attribute :runtime_status, :jsonbHash, default: {} attribute :runtime_auth_scopes, :jsonbArray, default: [] attribute :output_storage_classes, :jsonbArray, default: lambda { Rails.configuration.DefaultStorageClasses } + attribute :output_properties, :jsonbHash, default: {} serialize :environment, Hash serialize :mounts, Hash @@ -81,6 +82,7 @@ class Container < ArvadosModel t.add :gateway_address t.add :interactive_session_started t.add :output_storage_classes + t.add :output_properties end # Supported states for a container @@ -476,7 +478,7 @@ class Container < ArvadosModel def validate_change permitted = [:state] - progress_attrs = [:progress, :runtime_status, :log, :output] + progress_attrs = [:progress, :runtime_status, :log, :output, :output_properties] final_attrs = [:exit_code, :finished_at] if self.new_record? @@ -496,7 +498,7 @@ class Container < ArvadosModel permitted.push :priority when Running - permitted.push :priority, *progress_attrs + permitted.push :priority, :output_properties, *progress_attrs if self.state_changed? permitted.push :started_at, :gateway_address end diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb index bec3deb295..9116035905 100644 --- a/services/api/app/models/container_request.rb +++ b/services/api/app/models/container_request.rb @@ -24,6 +24,7 @@ class ContainerRequest < ArvadosModel attribute :properties, :jsonbHash, default: {} attribute :secret_mounts, :jsonbHash, default: {} attribute :output_storage_classes, :jsonbArray, default: lambda { Rails.configuration.DefaultStorageClasses } + attribute :output_properties, :jsonbHash, default: {} serialize :environment, Hash serialize :mounts, Hash @@ -78,6 +79,7 @@ class ContainerRequest < ArvadosModel t.add :state t.add :use_existing t.add :output_storage_classes + t.add :output_properties end # Supported states for a container request @@ -100,7 +102,7 @@ class ContainerRequest < ArvadosModel :output_path, :priority, :runtime_token, :runtime_constraints, :state, :container_uuid, :use_existing, :scheduling_parameters, :secret_mounts, :output_name, :output_ttl, - :output_storage_classes] + :output_storage_classes, :output_properties] def self.any_preemptible_instances? Rails.configuration.InstanceTypes.any? do |k, v| @@ -222,11 +224,7 @@ class ContainerRequest < ArvadosModel owner_uuid: self.owner_uuid, name: coll_name, manifest_text: "", - storage_classes_desired: self.output_storage_classes, - properties: { - 'type' => out_type, - 'container_request' => uuid, - }) + storage_classes_desired: self.output_storage_classes) end if out_type == "log" @@ -238,11 +236,28 @@ class ContainerRequest < ArvadosModel manifest = dst.manifest_text end + merged_properties = {} + merged_properties['container_request'] = uuid + + if out_type == 'output' and !requesting_container_uuid.nil? + # output of a child process, give it "intermediate" type by + # default. + merged_properties['type'] = 'intermediate' + else + merged_properties['type'] = out_type + end + + if out_type == "output" + merged_properties.update(container.output_properties) + merged_properties.update(self.output_properties) + end + coll.assign_attributes( portable_data_hash: Digest::MD5.hexdigest(manifest) + '+' + manifest.bytesize.to_s, manifest_text: manifest, trash_at: trash_at, - delete_at: trash_at) + delete_at: trash_at, + properties: merged_properties) coll.save_with_unique_name! self.send(out_type + '_uuid=', coll.uuid) end diff --git a/services/api/db/migrate/20220505112900_add_output_properties.rb b/services/api/db/migrate/20220505112900_add_output_properties.rb new file mode 100644 index 0000000000..7d8c4b1ffb --- /dev/null +++ b/services/api/db/migrate/20220505112900_add_output_properties.rb @@ -0,0 +1,31 @@ +# Copyright (C) The Arvados Authors. All rights reserved. +# +# SPDX-License-Identifier: AGPL-3.0 + +class AddOutputProperties < ActiveRecord::Migration[5.2] + def trgm_indexes + { + "container_requests" => "container_requests_trgm_text_search_idx", + } + end + + def up + add_column :container_requests, :output_properties, :jsonb, default: {} + add_column :containers, :output_properties, :jsonb, default: {} + + trgm_indexes.each do |model, indx| + execute "DROP INDEX IF EXISTS #{indx}" + execute "CREATE INDEX #{indx} ON #{model} USING gin((#{model.classify.constantize.full_text_trgm}) gin_trgm_ops)" + end + end + + def down + remove_column :container_requests, :output_properties + remove_column :containers, :output_properties + + trgm_indexes.each do |model, indx| + execute "DROP INDEX IF EXISTS #{indx}" + execute "CREATE INDEX #{indx} ON #{model} USING gin((#{model.classify.constantize.full_text_trgm}) gin_trgm_ops)" + end + end +end diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql index e6bba67625..c5f6d567bf 100644 --- a/services/api/db/structure.sql +++ b/services/api/db/structure.sql @@ -480,7 +480,8 @@ CREATE TABLE public.container_requests ( output_ttl integer DEFAULT 0 NOT NULL, secret_mounts jsonb DEFAULT '{}'::jsonb, runtime_token text, - output_storage_classes jsonb DEFAULT '["default"]'::jsonb + output_storage_classes jsonb DEFAULT '["default"]'::jsonb, + output_properties jsonb DEFAULT '{}'::jsonb ); @@ -543,7 +544,8 @@ CREATE TABLE public.containers ( lock_count integer DEFAULT 0 NOT NULL, gateway_address character varying, interactive_session_started boolean DEFAULT false NOT NULL, - output_storage_classes jsonb DEFAULT '["default"]'::jsonb + output_storage_classes jsonb DEFAULT '["default"]'::jsonb, + output_properties jsonb DEFAULT '{}'::jsonb ); @@ -1782,7 +1784,7 @@ CREATE INDEX container_requests_search_index ON public.container_requests USING -- Name: container_requests_trgm_text_search_idx; Type: INDEX; Schema: public; Owner: - -- -CREATE INDEX container_requests_trgm_text_search_idx ON public.container_requests USING gin (((((((((((((((((((((((((((((((((((((((((((COALESCE(uuid, ''::character varying))::text || ' '::text) || (COALESCE(owner_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(modified_by_client_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(modified_by_user_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(name, ''::character varying))::text) || ' '::text) || COALESCE(description, ''::text)) || ' '::text) || COALESCE((properties)::text, ''::text)) || ' '::text) || (COALESCE(state, ''::character varying))::text) || ' '::text) || (COALESCE(requesting_container_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(container_uuid, ''::character varying))::text) || ' '::text) || COALESCE(runtime_constraints, ''::text)) || ' '::text) || (COALESCE(container_image, ''::character varying))::text) || ' '::text) || COALESCE(environment, ''::text)) || ' '::text) || (COALESCE(cwd, ''::character varying))::text) || ' '::text) || COALESCE(command, ''::text)) || ' '::text) || (COALESCE(output_path, ''::character varying))::text) || ' '::text) || COALESCE(filters, ''::text)) || ' '::text) || COALESCE(scheduling_parameters, ''::text)) || ' '::text) || (COALESCE(output_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(log_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(output_name, ''::character varying))::text)) public.gin_trgm_ops); +CREATE INDEX container_requests_trgm_text_search_idx ON public.container_requests USING gin (((((((((((((((((((((((((((((((((((((((((((((COALESCE(uuid, ''::character varying))::text || ' '::text) || (COALESCE(owner_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(modified_by_client_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(modified_by_user_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(name, ''::character varying))::text) || ' '::text) || COALESCE(description, ''::text)) || ' '::text) || COALESCE((properties)::text, ''::text)) || ' '::text) || (COALESCE(state, ''::character varying))::text) || ' '::text) || (COALESCE(requesting_container_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(container_uuid, ''::character varying))::text) || ' '::text) || COALESCE(runtime_constraints, ''::text)) || ' '::text) || (COALESCE(container_image, ''::character varying))::text) || ' '::text) || COALESCE(environment, ''::text)) || ' '::text) || (COALESCE(cwd, ''::character varying))::text) || ' '::text) || COALESCE(command, ''::text)) || ' '::text) || (COALESCE(output_path, ''::character varying))::text) || ' '::text) || COALESCE(filters, ''::text)) || ' '::text) || COALESCE(scheduling_parameters, ''::text)) || ' '::text) || (COALESCE(output_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(log_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(output_name, ''::character varying))::text) || ' '::text) || COALESCE((output_properties)::text, ''::text))) public.gin_trgm_ops); -- @@ -3179,6 +3181,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20220224203102'), ('20220301155729'), ('20220303204419'), -('20220401153101'); +('20220401153101'), +('20220505112900'); diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb index aa649e9106..e5c0085184 100644 --- a/services/api/test/unit/container_request_test.rb +++ b/services/api/test/unit/container_request_test.rb @@ -469,13 +469,34 @@ class ContainerRequestTest < ActiveSupport::TestCase ].each do |token, expected, expected_priority| test "create as #{token} and expect requesting_container_uuid to be #{expected}" do set_user_from_auth token - cr = ContainerRequest.create(container_image: "img", output_path: "/tmp", command: ["echo", "foo"]) + cr = create_minimal_req! assert_not_nil cr.uuid, 'uuid should be set for newly created container_request' assert_equal expected, cr.requesting_container_uuid assert_equal expected_priority, cr.priority end end + [ + ['running_container_auth', 'zzzzz-dz642-runningcontainr', 501], + ].each do |token, expected, expected_priority| + test "create as #{token} with requesting_container_uuid set and expect output to be intermediate" do + set_user_from_auth token + cr = create_minimal_req! + assert_not_nil cr.uuid, 'uuid should be set for newly created container_request' + assert_equal expected, cr.requesting_container_uuid + assert_equal expected_priority, cr.priority + + cr.state = ContainerRequest::Committed + cr.save! + + run_container(cr) + cr.reload + output = Collection.find_by_uuid(cr.output_uuid) + props = {"type": "intermediate", "container_request": cr.uuid} + assert_equal props.symbolize_keys, output.properties.symbolize_keys + end + end + test "create as container_runtime_token and expect requesting_container_uuid to be zzzzz-dz642-20isqbkl8xwnsao" do set_user_from_auth :container_runtime_token Thread.current[:token] = "#{Thread.current[:token]}/zzzzz-dz642-20isqbkl8xwnsao" @@ -1448,4 +1469,46 @@ class ContainerRequestTest < ActiveSupport::TestCase assert_equal ["foo_storage_class"], output1.storage_classes_desired assert_equal ["bar_storage_class"], output2.storage_classes_desired end + + [ + [{}, {}, {"type": "output"}], + [{"a1": "b1"}, {}, {"type": "output", "a1": "b1"}], + [{}, {"a1": "b1"}, {"type": "output", "a1": "b1"}], + [{"a1": "b1"}, {"a1": "c1"}, {"type": "output", "a1": "b1"}], + [{"a1": "b1"}, {"a2": "c2"}, {"type": "output", "a1": "b1", "a2": "c2"}], + [{"type": "blah"}, {}, {"type": "blah"}], + ].each do |cr_prop, container_prop, expect_prop| + test "setting output_properties #{cr_prop} #{container_prop} on current container" do + act_as_user users(:active) do + cr = create_minimal_req!(priority: 1, + state: ContainerRequest::Committed, + output_name: 'foo', + output_properties: cr_prop) + + act_as_system_user do + logc = Collection.new(owner_uuid: system_user_uuid, + manifest_text: ". ef772b2f28e2c8ca84de45466ed19ee9+7815 0:0:arv-mount.txt\n") + logc.save! + + c = Container.find_by_uuid(cr.container_uuid) + c.update_attributes!(state: Container::Locked) + c.update_attributes!(state: Container::Running) + + c.update_attributes!(output_properties: container_prop) + + c.update_attributes!(state: Container::Complete, + exit_code: 0, + output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45', + log: logc.portable_data_hash) + logc.destroy + end + + cr.reload + expect_prop["container_request"] = cr.uuid + output = Collection.find_by_uuid(cr.output_uuid) + assert_equal expect_prop.symbolize_keys, output.properties.symbolize_keys + end + end + end + end