From a3d2b8e1de5b8c785846ddc57ae9a4c02bc51adc Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Tue, 10 May 2022 14:48:49 -0400 Subject: [PATCH] Update tests related to make_output_collection Also update API revision to reflect addition of output_properties Check API revision before adding output_properties. Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- sdk/cwl/arvados_cwl/arvcontainer.py | 14 ++++++++------ sdk/cwl/arvados_cwl/executor.py | 2 +- sdk/cwl/tests/test_make_output.py | 8 ++++---- sdk/cwl/tests/test_submit.py | 12 ++++++------ sdk/go/arvados/container.go | 1 + .../controllers/arvados/v1/schema_controller.rb | 2 +- 6 files changed, 21 insertions(+), 18 deletions(-) diff --git a/sdk/cwl/arvados_cwl/arvcontainer.py b/sdk/cwl/arvados_cwl/arvcontainer.py index b1784f8f3f..d53ec5c7f8 100644 --- a/sdk/cwl/arvados_cwl/arvcontainer.py +++ b/sdk/cwl/arvados_cwl/arvcontainer.py @@ -199,7 +199,7 @@ 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) - 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=intermediate_collection_info["trash_at"], @@ -342,13 +342,15 @@ class ArvadosContainer(JobBase): for pr in properties_req["processProperties"]: container_request["properties"][pr["propertyName"]] = self.builder.do_eval(pr["propertyValue"]) - container_request["output_properties"] = {} output_properties_req, _ = self.get_requirement("http://arvados.org/cwl#OutputCollectionProperties") if output_properties_req: - for pr in output_properties_req["outputProperties"]: - container_request["output_properties"][pr["propertyName"]] = self.builder.do_eval(pr["propertyValue"]) - - container_request["output_properties"].update(intermediate_collection_info["properties"]) + 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 server is too old to support setting properties on output collections.", + self.arvrunner.label(self)) if runtimeContext.runnerjob.startswith("arvwf:"): wfuuid = runtimeContext.runnerjob[6:runtimeContext.runnerjob.index("#")] diff --git a/sdk/cwl/arvados_cwl/executor.py b/sdk/cwl/arvados_cwl/executor.py index 1a4bf65a57..0bb17e99a2 100644 --- a/sdk/cwl/arvados_cwl/executor.py +++ b/sdk/cwl/arvados_cwl/executor.py @@ -792,7 +792,7 @@ The 'jobs' API is no longer supported. storage_classes = runtimeContext.storage_classes.strip().split(",") output_properties = {} - output_properties_req, _ = self.get_requirement("http://arvados.org/cwl#OutputCollectionProperties") + output_properties_req, _ = tool.get_requirement("http://arvados.org/cwl#OutputCollectionProperties") if output_properties_req: for pr in output_properties_req["outputProperties"]: output_properties[pr["propertyName"]] = self.builder.do_eval(pr["propertyValue"]) 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_submit.py b/sdk/cwl/tests/test_submit.py index 305d51e144..2b74435f7e 100644 --- a/sdk/cwl/tests/test_submit.py +++ b/sdk/cwl/tests/test_submit.py @@ -569,7 +569,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 +578,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 +591,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 +600,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 +612,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 +621,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 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/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, -- 2.30.2