From 79786a56410ef381499fb0bfdc5a18407ab33082 Mon Sep 17 00:00:00 2001 From: Jiayong Li Date: Thu, 10 Nov 2016 11:55:16 -0500 Subject: [PATCH] Change output_tags to an attribute instead of kwarg, modify tests --- sdk/cwl/arvados_cwl/__init__.py | 15 +++++++++------ sdk/cwl/arvados_cwl/arvcontainer.py | 4 ++-- sdk/cwl/arvados_cwl/arvjob.py | 7 ++++--- sdk/cwl/arvados_cwl/crunch_script.py | 5 +++-- sdk/cwl/arvados_cwl/runner.py | 3 ++- sdk/cwl/tests/test_make_output.py | 8 ++++++-- sdk/cwl/tests/test_submit.py | 23 ++++++++++++++++++++++- 7 files changed, 48 insertions(+), 17 deletions(-) diff --git a/sdk/cwl/arvados_cwl/__init__.py b/sdk/cwl/arvados_cwl/__init__.py index a22b075ce4..501bad5c5d 100644 --- a/sdk/cwl/arvados_cwl/__init__.py +++ b/sdk/cwl/arvados_cwl/__init__.py @@ -49,7 +49,7 @@ class ArvCwlRunner(object): """ - def __init__(self, api_client, work_api=None, keep_client=None, output_name=None): + def __init__(self, api_client, work_api=None, keep_client=None, output_name=None, output_tags=None): self.api = api_client self.processes = {} self.lock = threading.Lock() @@ -64,6 +64,7 @@ class ArvCwlRunner(object): self.pipeline = None self.final_output_collection = None self.output_name = output_name + self.output_tags = output_tags self.project_uuid = None if keep_client is not None: @@ -183,7 +184,7 @@ class ArvCwlRunner(object): for v in obj: self.check_writable(v) - def make_output_collection(self, name, outputObj, tagsString): + def make_output_collection(self, name, tagsString, outputObj): outputObj = copy.deepcopy(outputObj) files = [] @@ -310,9 +311,9 @@ class ArvCwlRunner(object): self.output_callback, **kwargs).next() else: - runnerjob = RunnerContainer(self, tool, job_order, kwargs.get("enable_reuse"), self.output_name) + runnerjob = RunnerContainer(self, tool, job_order, kwargs.get("enable_reuse"), self.output_name, self.output_tags) else: - runnerjob = RunnerJob(self, tool, job_order, kwargs.get("enable_reuse"), self.output_name) + runnerjob = RunnerJob(self, tool, job_order, kwargs.get("enable_reuse"), self.output_name, self.output_tags) if not kwargs.get("submit") and "cwl_runner_job" not in kwargs and not self.work_api == "containers": # Create pipeline for local run @@ -395,7 +396,9 @@ class ArvCwlRunner(object): else: if self.output_name is None: self.output_name = "Output of %s" % (shortname(tool.tool["id"])) - self.make_output_collection(self.output_name, self.final_output, kwargs.get("output_tags", "")) + if self.output_tags is None: + self.output_tags = "" + self.make_output_collection(self.output_name, self.output_tags, self.final_output) self.set_crunch_output() if self.final_status != "success": @@ -511,7 +514,7 @@ def main(args, stdout, stderr, api_client=None, keep_client=None): try: if api_client is None: api_client=arvados.api('v1', model=OrderedJsonModel()) - runner = ArvCwlRunner(api_client, work_api=arvargs.work_api, keep_client=keep_client, output_name=arvargs.output_name) + runner = ArvCwlRunner(api_client, work_api=arvargs.work_api, keep_client=keep_client, output_name=arvargs.output_name, output_tags=arvargs.output_tags) except Exception as e: logger.error(e) return 1 diff --git a/sdk/cwl/arvados_cwl/arvcontainer.py b/sdk/cwl/arvados_cwl/arvcontainer.py index cae9027c48..afcc29a21a 100644 --- a/sdk/cwl/arvados_cwl/arvcontainer.py +++ b/sdk/cwl/arvados_cwl/arvcontainer.py @@ -189,8 +189,8 @@ class RunnerContainer(Runner): if self.output_name: command.append("--output-name=" + self.output_name) - if kwargs.get("output_tags"): - command.append("--output-tags=" + kwargs.get("output_tags")) + if self.output_tags: + command.append("--output-tags=" + self.output_tags) if self.enable_reuse: command.append("--enable-reuse") diff --git a/sdk/cwl/arvados_cwl/arvjob.py b/sdk/cwl/arvados_cwl/arvjob.py index 3d99391449..f48d8bbe11 100644 --- a/sdk/cwl/arvados_cwl/arvjob.py +++ b/sdk/cwl/arvados_cwl/arvjob.py @@ -239,8 +239,8 @@ class RunnerJob(Runner): if self.output_name: self.job_order["arv:output_name"] = self.output_name - if kwargs.get("output_tags"): - self.job_order["arv:output_tags"] = kwargs.get("output_tags") + if self.output_tags: + self.job_order["arv:output_tags"] = self.output_tags self.job_order["arv:enable_reuse"] = self.enable_reuse @@ -309,7 +309,8 @@ class RunnerTemplate(object): tool=tool, job_order=job_order, enable_reuse=enable_reuse, - output_name=None) + output_name=None, + output_tags=None) def pipeline_component_spec(self): """Return a component that Workbench and a-r-p-i will understand. diff --git a/sdk/cwl/arvados_cwl/crunch_script.py b/sdk/cwl/arvados_cwl/crunch_script.py index cc9d87899c..849b177aeb 100644 --- a/sdk/cwl/arvados_cwl/crunch_script.py +++ b/sdk/cwl/arvados_cwl/crunch_script.py @@ -63,13 +63,14 @@ def run(): adjustDirObjs(job_order_object, functools.partial(getListing, arvados_cwl.fsaccess.CollectionFsAccess("", api_client=api))) output_name = None + output_tags = None enable_reuse = True if "arv:output_name" in job_order_object: output_name = job_order_object["arv:output_name"] del job_order_object["arv:output_name"] if "arv:output_tags" in job_order_object: - args.output_tags = job_order_object["arv:output_tags"] + output_tags = job_order_object["arv:output_tags"] del job_order_object["arv:output_tags"] if "arv:enable_reuse" in job_order_object: @@ -77,7 +78,7 @@ def run(): del job_order_object["arv:enable_reuse"] runner = arvados_cwl.ArvCwlRunner(api_client=arvados.api('v1', model=OrderedJsonModel()), - output_name=output_name) + output_name=output_name, output_tags=output_tags) t = load_tool(job_order_object, runner.arv_make_tool) diff --git a/sdk/cwl/arvados_cwl/runner.py b/sdk/cwl/arvados_cwl/runner.py index 6f157db3c6..2b5d186843 100644 --- a/sdk/cwl/arvados_cwl/runner.py +++ b/sdk/cwl/arvados_cwl/runner.py @@ -143,7 +143,7 @@ def arvados_jobs_image(arvrunner): return img class Runner(object): - def __init__(self, runner, tool, job_order, enable_reuse, output_name): + def __init__(self, runner, tool, job_order, enable_reuse, output_name, output_tags): self.arvrunner = runner self.tool = tool self.job_order = job_order @@ -152,6 +152,7 @@ class Runner(object): self.uuid = None self.final_output = None self.output_name = output_name + self.output_tags = output_tags def update_pipeline_component(self, record): pass diff --git a/sdk/cwl/tests/test_make_output.py b/sdk/cwl/tests/test_make_output.py index 0a33649adb..d818ccac6f 100644 --- a/sdk/cwl/tests/test_make_output.py +++ b/sdk/cwl/tests/test_make_output.py @@ -27,12 +27,14 @@ class TestMakeOutput(unittest.TestCase): readermock = mock.MagicMock() reader.return_value = readermock + final_uuid = final.manifest_locater() + cwlout = StringIO.StringIO() openmock = mock.MagicMock() final.open.return_value = openmock openmock.__enter__.return_value = cwlout - runner.make_output_collection("Test output", { + runner.make_output_collection("Test output", "tag0", { "foo": { "class": "File", "location": "keep:99999999999999999999999999999991+99/foo.txt", @@ -44,7 +46,7 @@ class TestMakeOutput(unittest.TestCase): "location": "keep:99999999999999999999999999999992+99/bar.txt", "basename": "baz.txt" } - }, "") + }) 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)]) @@ -61,3 +63,5 @@ class TestMakeOutput(unittest.TestCase): }""", cwlout.getvalue()) self.assertIs(final, runner.final_output_collection) + self.assertIs(final_uuid, runner.final_output_collection.manifest_locater()) + self.api.links().create.assert_has_calls([mock.call(body={"head_uuid": final_uuid, "link_class": "tag", "name": "tag0"}), mock.call().execute()]) diff --git a/sdk/cwl/tests/test_submit.py b/sdk/cwl/tests/test_submit.py index 2371c33370..ebb8360830 100644 --- a/sdk/cwl/tests/test_submit.py +++ b/sdk/cwl/tests/test_submit.py @@ -269,6 +269,27 @@ class TestSubmit(unittest.TestCase): self.assertEqual(capture_stdout.getvalue(), stubs.expect_pipeline_uuid + '\n') + @mock.patch("time.sleep") + @stubs + def test_submit_output_name(self, stubs, tm): + output_name = "test_output_name" + + capture_stdout = cStringIO.StringIO() + exited = arvados_cwl.main( + ["--submit", "--no-wait", "--debug", "--output-name", output_name, + "tests/wf/submit_wf.cwl", "tests/submit_test_job.json"], + capture_stdout, sys.stderr, api_client=stubs.api) + self.assertEqual(exited, 0) + + stubs.expect_pipeline_instance["components"]["cwl-runner"]["script_parameters"]["arv:output_name"] = output_name + + expect_pipeline = copy.deepcopy(stubs.expect_pipeline_instance) + expect_pipeline["owner_uuid"] = stubs.fake_user_uuid + stubs.api.pipeline_instances().create.assert_called_with( + body=expect_pipeline) + self.assertEqual(capture_stdout.getvalue(), + stubs.expect_pipeline_uuid + '\n') + @mock.patch("time.sleep") @stubs def test_submit_output_tags(self, stubs, tm): @@ -281,7 +302,7 @@ class TestSubmit(unittest.TestCase): capture_stdout, sys.stderr, api_client=stubs.api) self.assertEqual(exited, 0) - stubs.expect_pipeline_instance["components"]["cwl-runner"]["script_parameters"]["arv:output_tags"] = "tag0,tag1,tag2" + stubs.expect_pipeline_instance["components"]["cwl-runner"]["script_parameters"]["arv:output_tags"] = output_tags expect_pipeline = copy.deepcopy(stubs.expect_pipeline_instance) expect_pipeline["owner_uuid"] = stubs.fake_user_uuid -- 2.30.2