Directory output fixes for PAPI and 'version development' WDL [CROM-6…

…875] (broadinstitute#6678)
takeshi-yoshimura · Feb 14, 2022 · 2e46ca2 · 2e46ca2
1 parent c785a0a
commit 2e46ca2
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 2 deletions.
diff --git a/centaur/src/main/resources/standardTestCases/directory_type_output_papi.test b/centaur/src/main/resources/standardTestCases/directory_type_output_papi.test
@@ -0,0 +1,12 @@
+name: directory_type_output_papi
+testFormat: workflowsuccess
+tags: ["wdl_biscayne"]
+backends: [Papi]
+
+files {
+  workflow: wdl_biscayne/directory_type_output/directory_type_output.wdl
+}
+
+metadata {
+  workflowName: main
+}
diff --git a/.../resources/standardTestCases/wdl_biscayne/directory_type_output/directory_type_output.wdl b/.../resources/standardTestCases/wdl_biscayne/directory_type_output/directory_type_output.wdl
@@ -0,0 +1,54 @@
+version development
+
+# CROM-6875 repro WDL to exercise Directory outputs. Since the Directory type does not exist in WDL versions 1.0 or
+# draft-2, the bug this is checking for cannot and does not exist in those WDL versions.
+workflow main {
+    call main { input: s1 = "x", s2 = "y" }
+    scatter (f in main.f) {
+        call checker { input: f = f }
+    }
+    output { Array[File] f = main.f }
+}
+
+task main {
+    input {
+        String s1
+        String s2
+    }
+
+    command <<<
+        set -euo pipefail
+        mkdir d
+        touch "d/~{s1}"
+        touch "d/~{s2}"
+        echo -e "d/~{s1}\nd/~{s2}"
+    >>>
+
+    output {
+        Directory d = "d"
+        Array[File] f = read_lines(stdout())
+    }
+
+    runtime {
+        docker: "debian:stable-slim"
+    }
+}
+
+task checker {
+    # Check files were actually created as expected above
+    input {
+        File f
+    }
+
+    command <<<
+        set -euo pipefail
+        [ -f ~{f} ]
+    >>>
+
+    output {
+    }
+
+    runtime {
+        docker: "debian:stable-slim"
+    }
+}
diff --git a/...romwell/backend/google/pipelines/v2alpha1/PipelinesApiAsyncBackendJobExecutionActor.scala b/...romwell/backend/google/pipelines/v2alpha1/PipelinesApiAsyncBackendJobExecutionActor.scala
@@ -292,7 +292,7 @@ class PipelinesApiAsyncBackendJobExecutionActor(standardParams: StandardAsyncExe
       jesOutputs collectFirst {
         case jesOutput if jesOutput.name == makeSafeReferenceName(path) =>
           val pathAsString = jesOutput.cloudPath.pathAsString
-          if (!jesOutput.cloudPath.exists) {
+          if (jesOutput.isFileParameter && !jesOutput.cloudPath.exists) {
             // This is not an error if the path represents a `File?` optional output (the PAPI delocalization script
             // should have failed if this file output was not optional but missing). Throw to produce the correct "empty
             // optional" value for a missing optional file output.

diff --git a/.../cromwell/backend/google/pipelines/v2beta/PipelinesApiAsyncBackendJobExecutionActor.scala b/.../cromwell/backend/google/pipelines/v2beta/PipelinesApiAsyncBackendJobExecutionActor.scala
@@ -292,7 +292,7 @@ class PipelinesApiAsyncBackendJobExecutionActor(standardParams: StandardAsyncExe
       jesOutputs collectFirst {
         case jesOutput if jesOutput.name == makeSafeReferenceName(path) =>
           val pathAsString = jesOutput.cloudPath.pathAsString
-          if (!jesOutput.cloudPath.exists) {
+          if (jesOutput.isFileParameter && !jesOutput.cloudPath.exists) {
             // This is not an error if the path represents a `File?` optional output (the PAPI delocalization script
             // should have failed if this file output was not optional but missing). Throw to produce the correct "empty
             // optional" value for a missing optional file output.