[ARVADOS] created: 1.3.0-2142-g02dbc92e0

Git user git at public.arvados.org
Sun Feb 9 23:07:36 UTC 2020


        at  02dbc92e0a0df291a97edcfcc419319fdbc5f750 (commit)


commit 02dbc92e0a0df291a97edcfcc419319fdbc5f750
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Sun Feb 9 18:07:15 2020 -0500

    16139: Add test
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/sdk/cwl/setup.py b/sdk/cwl/setup.py
index aa68933c6..62ceab2fa 100644
--- a/sdk/cwl/setup.py
+++ b/sdk/cwl/setup.py
@@ -60,7 +60,7 @@ setup(name='arvados-cwl-runner',
       ],
       test_suite='tests',
       tests_require=[
-          'mock>=1.0',
+          'mock>=1.0,<4',
           'subprocess32>=3.5.1',
       ],
       zip_safe=True
diff --git a/sdk/cwl/tests/test_submit.py b/sdk/cwl/tests/test_submit.py
index 927e43ad7..397ae1422 100644
--- a/sdk/cwl/tests/test_submit.py
+++ b/sdk/cwl/tests/test_submit.py
@@ -388,6 +388,19 @@ class TestSubmit(unittest.TestCase):
                          stubs.expect_container_request_uuid + '\n')
         self.assertEqual(exited, 0)
 
+
+    @stubs
+    def test_submit_container_tool(self, stubs):
+        # test for issue #16139
+        exited = arvados_cwl.main(
+            ["--submit", "--no-wait", "--api=containers", "--debug",
+                "tests/tool/tool_with_sf.cwl", "tests/tool/tool_with_sf.yml"],
+            stubs.capture_stdout, sys.stderr, api_client=stubs.api, keep_client=stubs.keep_client)
+
+        self.assertEqual(stubs.capture_stdout.getvalue(),
+                         stubs.expect_container_request_uuid + '\n')
+        self.assertEqual(exited, 0)
+
     @stubs
     def test_submit_container_no_reuse(self, stubs):
         exited = arvados_cwl.main(
diff --git a/sdk/cwl/tests/tool/blub.txt.cat b/sdk/cwl/tests/tool/blub.txt.cat
new file mode 100644
index 000000000..d7c42215c
--- /dev/null
+++ b/sdk/cwl/tests/tool/blub.txt.cat
@@ -0,0 +1 @@
+clipper clupper
diff --git a/sdk/cwl/tests/tool/tool_with_sf.cwl b/sdk/cwl/tests/tool/tool_with_sf.cwl
new file mode 100644
index 000000000..0beb7ad78
--- /dev/null
+++ b/sdk/cwl/tests/tool/tool_with_sf.cwl
@@ -0,0 +1,24 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+# Test case for arvados-cwl-runner
+#
+# Used to test whether scanning a tool file for dependencies (e.g. default
+# value blub.txt) and uploading to Keep works as intended.
+
+class: CommandLineTool
+cwlVersion: v1.0
+requirements:
+  - class: DockerRequirement
+    dockerPull: debian:8
+inputs:
+  - id: x
+    type: File
+    secondaryFiles:
+      - .cat
+    inputBinding:
+      valueFrom: $(self.path).cat
+      position: 1
+outputs: []
+baseCommand: cat
diff --git a/sdk/cwl/tests/tool/tool_with_sf.yml b/sdk/cwl/tests/tool/tool_with_sf.yml
new file mode 100644
index 000000000..3f79d5738
--- /dev/null
+++ b/sdk/cwl/tests/tool/tool_with_sf.yml
@@ -0,0 +1,3 @@
+x:
+  class: File
+  location: blub.txt
diff --git a/sdk/python/setup.py b/sdk/python/setup.py
index 87977c218..ff68e2a7f 100644
--- a/sdk/python/setup.py
+++ b/sdk/python/setup.py
@@ -64,6 +64,6 @@ setup(name='arvados-python-client',
           'Programming Language :: Python :: 3',
       ],
       test_suite='tests',
-      tests_require=['pbr<1.7.0', 'mock>=1.0', 'PyYAML'],
+      tests_require=['pbr<1.7.0', 'mock>=1.0,<4', 'PyYAML'],
       zip_safe=False
       )

commit 6272cb13e54254ad5ab3da898716daaab3ddae6a
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Fri Feb 7 15:59:10 2020 -0500

    16139: Fix secondaryFile errors when running --submit --no-wait
    
    When submitting, we want to preserve the original CWL version of the
    document.
    
    However, when it creates a RunnerContainer (a cwltool.Process) it
    examines the input interface and expects it to be in the 1.1 data
    model.  But if we preserve/reload the original document, it is still
    in the 1.0 data model.  This causes
    
    The code was deliberately overriding the CWL version in metadata to
    make this work.  This results in the problem reported on this ticket.
    
    The fix is to maintain both the updated and preserved documents, and
    use them appropriately where they are expected.
    
    refs #16139
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/sdk/cwl/arvados_cwl/executor.py b/sdk/cwl/arvados_cwl/executor.py
index 406ebfd2d..99d4c4e9a 100644
--- a/sdk/cwl/arvados_cwl/executor.py
+++ b/sdk/cwl/arvados_cwl/executor.py
@@ -521,10 +521,10 @@ The 'jobs' API is no longer supported.
             for req in job_reqs:
                 tool.requirements.append(req)
 
-    def arv_executor(self, tool, job_order, runtimeContext, logger=None):
+    def arv_executor(self, updated_tool, job_order, runtimeContext, logger=None):
         self.debug = runtimeContext.debug
 
-        tool.visit(self.check_features)
+        updated_tool.visit(self.check_features)
 
         self.project_uuid = runtimeContext.project_uuid
         self.pipeline = None
@@ -545,16 +545,20 @@ The 'jobs' API is no longer supported.
             raise Exception("--submit-request-uuid requires containers API, but using '{}' api".format(self.work_api))
 
         if not runtimeContext.name:
-            runtimeContext.name = self.name = tool.tool.get("label") or tool.metadata.get("label") or os.path.basename(tool.tool["id"])
+            runtimeContext.name = self.name = updated_tool.tool.get("label") or updated_tool.metadata.get("label") or os.path.basename(updated_tool.tool["id"])
 
         # Upload local file references in the job order.
         job_order = upload_job_order(self, "%s input" % runtimeContext.name,
-                                     tool, job_order)
+                                     updated_tool, job_order)
+
+        # the last clause means: if it is a command line tool, and we
+        # are going to wait for the result, and always_submit_runner
+        # is false, then we don't submit a runner process.
 
         submitting = (runtimeContext.update_workflow or
                       runtimeContext.create_workflow or
                       (runtimeContext.submit and not
-                       (tool.tool["class"] == "CommandLineTool" and
+                       (updated_tool.tool["class"] == "CommandLineTool" and
                         runtimeContext.wait and
                         not runtimeContext.always_submit_runner)))
 
@@ -564,8 +568,11 @@ The 'jobs' API is no longer supported.
         if submitting:
             # Document may have been auto-updated. Reload the original
             # document with updating disabled because we want to
-            # submit the original document, not the auto-updated one.
-            tool = load_tool(tool.tool["id"], loadingContext)
+            # submit the document with its original CWL version, not
+            # the auto-updated one.
+            tool = load_tool(updated_tool.tool["id"], loadingContext)
+        else:
+            tool = updated_tool
 
         # Upload direct dependencies of workflow steps, get back mapping of files to keep references.
         # Also uploads docker images.
@@ -632,22 +639,23 @@ The 'jobs' API is no longer supported.
         if runtimeContext.submit:
             # Submit a runner job to run the workflow for us.
             if self.work_api == "containers":
-                if tool.tool["class"] == "CommandLineTool" and runtimeContext.wait and (not runtimeContext.always_submit_runner):
-                    runtimeContext.runnerjob = tool.tool["id"]
+                if submitting:
+                    tool = RunnerContainer(self, updated_tool,
+                                           tool, loadingContext, runtimeContext.enable_reuse,
+                                           self.output_name,
+                                           self.output_tags,
+                                           submit_runner_ram=runtimeContext.submit_runner_ram,
+                                           name=runtimeContext.name,
+                                           on_error=runtimeContext.on_error,
+                                           submit_runner_image=runtimeContext.submit_runner_image,
+                                           intermediate_output_ttl=runtimeContext.intermediate_output_ttl,
+                                           merged_map=merged_map,
+                                           priority=runtimeContext.priority,
+                                           secret_store=self.secret_store,
+                                           collection_cache_size=runtimeContext.collection_cache_size,
+                                           collection_cache_is_default=self.should_estimate_cache_size)
                 else:
-                    tool = RunnerContainer(self, tool, loadingContext, runtimeContext.enable_reuse,
-                                                self.output_name,
-                                                self.output_tags,
-                                                submit_runner_ram=runtimeContext.submit_runner_ram,
-                                                name=runtimeContext.name,
-                                                on_error=runtimeContext.on_error,
-                                                submit_runner_image=runtimeContext.submit_runner_image,
-                                                intermediate_output_ttl=runtimeContext.intermediate_output_ttl,
-                                                merged_map=merged_map,
-                                                priority=runtimeContext.priority,
-                                                secret_store=self.secret_store,
-                                                collection_cache_size=runtimeContext.collection_cache_size,
-                                                collection_cache_is_default=self.should_estimate_cache_size)
+                    runtimeContext.runnerjob = tool.tool["id"]
 
         if runtimeContext.cwl_runner_job is not None:
             self.uuid = runtimeContext.cwl_runner_job.get('uuid')
diff --git a/sdk/cwl/arvados_cwl/runner.py b/sdk/cwl/arvados_cwl/runner.py
index 19a6dd98b..2239e0f9d 100644
--- a/sdk/cwl/arvados_cwl/runner.py
+++ b/sdk/cwl/arvados_cwl/runner.py
@@ -578,7 +578,8 @@ class Runner(Process):
     """Base class for runner processes, which submit an instance of
     arvados-cwl-runner and wait for the final result."""
 
-    def __init__(self, runner, tool, loadingContext, enable_reuse,
+    def __init__(self, runner, updated_tool,
+                 tool, loadingContext, enable_reuse,
                  output_name, output_tags, submit_runner_ram=0,
                  name=None, on_error=None, submit_runner_image=None,
                  intermediate_output_ttl=0, merged_map=None,
@@ -587,10 +588,9 @@ class Runner(Process):
                  collection_cache_is_default=True):
 
         loadingContext = loadingContext.copy()
-        loadingContext.metadata = loadingContext.metadata.copy()
-        loadingContext.metadata["cwlVersion"] = INTERNAL_VERSION
+        loadingContext.metadata = updated_tool.metadata.copy()
 
-        super(Runner, self).__init__(tool.tool, loadingContext)
+        super(Runner, self).__init__(updated_tool.tool, loadingContext)
 
         self.arvrunner = runner
         self.embedded_tool = tool

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list