Merge branch 'develop'

.github/issue_template.md

@@ -1,7 +1,15 @@
+###
+### IMPORTANT: Please file new issues over in our Jira issue tracker!
+###
+### https://broadworkbench.atlassian.net/projects/BA/issues
+###
+### You may need to create an account before you can view/create issues.
+###
+
 <!--
 Hi!  Thanks for taking the time to report feedback.
 
-Before posting an issue here, please check whether your question is already answered in our:
+Before posting an issue over in Jira tracker, please check whether your question is already answered in our:
   forum  https://gatkforums.broadinstitute.org/wdl/categories/ask-the-wdl-team
   documentation http://cromwell.readthedocs.io/en/develop/
 
@@ -11,7 +19,9 @@ WDL https://gatkforums.broadinstitute.org/wdl/categories/ask-the-wdl-team
 CWL https://www.biostars.org/
 -->
 
-<!-- Are you seeing something that looks like a bug?  Then great!  You're in the right place.  -->
+<!-- Are you seeing something that looks like a bug?  Then great!  You're almost in the right place.  -->
+
+<!-- You'll want to go to https://broadworkbench.atlassian.net/projects/BA/issues and then tell us: -->
 
 <!-- Which backend are you running?  -->
 

CHANGELOG.md

@@ -1,5 +1,47 @@
 # Cromwell Change Log
 
+## 42 Release Notes
+
+### Womtool endpoint
+
+The `/describe` endpoint now differentiates between an invalid workflow and a valid workflow with invalid inputs.
+
+Specifically, the new `validWorkflow` key indicates whether the workflow file is valid by itself. If inputs are provided, they are not considered when calculating this field; if inputs are not provided, the value is identical to `valid`.
+
+### Configuration Changes
+
+ *  Virtual private networks can now be configured. See the section below for details.
+
+#### Batch Request Timeouts
+
+The timeout on Cromwell's requests to PAPIv2 can now be configured. See the sample PAPIv2.conf for more documentation:
+
+```conf
+backend {
+  providers {
+    PAPIv2 {
+      config { 
+        batch-requests {
+          timeouts {
+            read = 10 seconds
+            connect = 10 seconds
+          }
+        }
+      }
+    }
+  }
+}
+```
+
+### Virtual Private Networks
+
+Cromwell now allows PAPIV2 jobs to run on a private network by adding the network name inside `virtual-private-cloud` in backend configuration.
+More info [here](https://cromwell.readthedocs.io/en/stable/backends/Google/).
+
+### AWS Backend
+
+Now includes background job status polling to hopefully reduce the incidence of 'HTTP 429' errors for large workflows.
+
 ## 41 Release Notes
 
 ### Workflow Options

CromIAM/src/main/resources/swagger/cromiam.yaml

@@ -1005,7 +1005,7 @@ definitions:
     properties:
       valid:
         type: boolean
-        description: Whether the workflow and inputs as submitted passed validation
+        description: Indicates that the workflow is valid and that the inputs, if provided, are compatible with the workflow.
       errors:
         type: array
         items:
@@ -1015,6 +1015,9 @@ definitions:
           - "We might also provide warnings to a 'valid' workflow here"
           - "Otherwise, 'errors' will be the empty array"
         description: The set of validation failure messages
+      validWorkflow:
+        type: boolean
+        description: Indicates whether the workflow file is valid by itself. If inputs are provided, they are not considered when calculating this field; if inputs are not provided, the value is identical to `valid`.
       name:
         type: string
         description: For a source file with one workflow and zero or more tasks, the name of the workflow. For a single task, the name of the task. For a source file with multiple tasks but no workflows, the empty string.

README.md

@@ -11,4 +11,10 @@ First time to Cromwell? Get started with [Tutorials](http://cromwell.readthedocs
 
 Thinking about contributing to Cromwell? Get started by reading our [Contributor Guide](CONTRIBUTING.md).
 
+### Issue tracking is now on JIRA
+
+Need to file an issue? Head over to [our JIRA](https://broadworkbench.atlassian.net/projects/BA/issues). You can sign in with any Google account. 
+
+As of May 2019, we are in the process of migrating all issues from Github to JIRA. At a later date to be announced, submitting new Github issues will be disabled.
+
 ![Jamie, the Cromwell pig](docs/jamie_the_cromwell_pig.png)

backend/src/main/scala/cromwell/backend/standard/StandardSyncExecutionActor.scala

@@ -10,6 +10,7 @@ import cromwell.core.Dispatcher
 import cromwell.services.keyvalue.KeyValueServiceActor._
 
 import scala.concurrent.{Future, Promise}
+import scala.util.control.NoStackTrace
 
 trait StandardSyncExecutionActorParams extends StandardJobExecutionActorParams {
   /** The class for creating an async backend. */
@@ -180,7 +181,7 @@ class StandardSyncExecutionActor(val standardParams: StandardSyncExecutionActorP
   def jobFailingDecider: Decider = {
     case exception: Exception =>
       completionPromise.tryFailure(
-        new RuntimeException(s"${createAsyncRefName()} failed and didn't catch its exception.", exception))
+        new RuntimeException(s"${createAsyncRefName()} failed and didn't catch its exception. This condition has been handled and the job will be marked as failed.", exception) with NoStackTrace)
       Stop
   }
 

backend/src/main/scala/cromwell/backend/validation/ValidatedRuntimeAttributesBuilder.scala

@@ -10,6 +10,8 @@ import wom.expression.WomExpression
 import wom.types.WomType
 import wom.values.WomValue
 
+import scala.util.control.NoStackTrace
+
 final case class ValidatedRuntimeAttributes(attributes: Map[String, Any])
 
 /**
@@ -59,7 +61,7 @@ trait ValidatedRuntimeAttributesBuilder {
     val runtimeAttributesErrorOr: ErrorOr[ValidatedRuntimeAttributes] = validate(attrs)
     runtimeAttributesErrorOr match {
       case Valid(runtimeAttributes) => runtimeAttributes
-      case Invalid(nel) => throw new RuntimeException with MessageAggregation {
+      case Invalid(nel) => throw new RuntimeException with MessageAggregation with NoStackTrace {
         override def exceptionContext: String = "Runtime attribute validation failed"
 
         override def errorMessages: Traversable[String] = nel.toList

centaur/src/it/scala/centaur/AbstractCentaurTestCaseSpec.scala

@@ -15,7 +15,7 @@ import org.scalatest._
 import scala.concurrent.Future
 
 @DoNotDiscover
-abstract class AbstractCentaurTestCaseSpec(cromwellBackends: List[String]) extends AsyncFlatSpec with Matchers {
+abstract class AbstractCentaurTestCaseSpec(cromwellBackends: List[String], cromwellTracker: Option[CromwellTracker] = None) extends AsyncFlatSpec with Matchers {
 
   /*
   NOTE: We need to statically initialize the object so that the exceptions appear here in the class constructor.
@@ -27,7 +27,7 @@ abstract class AbstractCentaurTestCaseSpec(cromwellBackends: List[String]) exten
 
   private def testCases(baseFile: File): List[CentaurTestCase] = {
     val files = baseFile.list.filter(_.isRegularFile).toList
-    val testCases = files.traverse(CentaurTestCase.fromFile)
+    val testCases = files.traverse(CentaurTestCase.fromFile(cromwellTracker))
 
     testCases match {
       case Valid(l) => l
@@ -95,7 +95,7 @@ abstract class AbstractCentaurTestCaseSpec(cromwellBackends: List[String]) exten
         testName = testCase.workflow.testName + " (draft-2 to 1.0 upgrade)",
         data = testCase.workflow.data.copy(
           workflowContent = Option(upgradeResult.stdout.get), // this '.get' catches an error if upgrade fails
-          zippedImports = Option(upgradedImportsDir.zip())))) // An empty zip appears to be completely harmless, so no special handling
+          zippedImports = Option(upgradedImportsDir.zip()))))(cromwellTracker) // An empty zip appears to be completely harmless, so no special handling
 
     rootWorkflowFile.delete(true)
     upgradedImportsDir.delete(true)

centaur/src/it/scala/centaur/CentaurTestSuite.scala

@@ -2,12 +2,14 @@ package centaur
 
 import centaur.api.CentaurCromwellClient
 import centaur.test.standard.CentaurTestCase
-import centaur.test.standard.CentaurTestFormat.{InstantAbort, RestartFormat, ScheduledAbort}
+import com.typesafe.config.{Config, ConfigFactory}
+import com.typesafe.scalalogging.StrictLogging
+import net.ceedubs.ficus.Ficus._
 import org.scalatest.{BeforeAndAfterAll, ParallelTestExecution, Suite, Suites}
 
 import scala.sys.ShutdownHookThread
 
-object CentaurTestSuite {
+object CentaurTestSuite extends StrictLogging {
   // Start cromwell if we're in Managed mode
   // Note: we can't use beforeAll to start Cromwell, because beforeAll is executed once the suite is instantiated and the
   // tests exist. However because the set of tests differs depending on the backends supported by Cromwell, it needs to be up
@@ -23,19 +25,35 @@ object CentaurTestSuite {
   }
 
   val cromwellBackends = CentaurCromwellClient.backends.unsafeRunSync().supportedBackends.map(_.toLowerCase)
-
-  def runSequential(testCase: CentaurTestCase): Boolean = testCase.testFormat match {
-    case _: RestartFormat| _: ScheduledAbort | InstantAbort => true
-    case _ => false
-  }
 
   def isWdlUpgradeTest(testCase: CentaurTestCase): Boolean = testCase.containsTag("wdl_upgrade")
 
   def isEngineUpgradeTest(testCase: CentaurTestCase): Boolean = testCase.containsTag("engine_upgrade")
 
   def isPapiUpgradeTest(testCase: CentaurTestCase): Boolean = testCase.containsTag("papi_upgrade")
 
-  def runParallel(testCase: CentaurTestCase) = !runSequential(testCase)
+  /** Horicromtality-related assertion config. */
+  val cromwellTracker: Option[CromwellTracker] = {
+
+    def backendCountFromConfig(config: Config): Option[Int] = {
+      val assert = config.getOrElse("assert", default = false)
+      val backendCount = config.as[Option[Int]]("backend-count")
+      (assert, backendCount) match {
+        case (false, _) => None
+        case (true, Some(_)) => backendCount
+        case (true, _) =>
+          val message = "Invalid Centaur configuration: `horicromtal` must define `backend-count` if `assert = true`"
+          throw new RuntimeException(message)
+      }
+    }
+
+    for {
+      config <- ConfigFactory.load().as[Option[Config]]("centaur.horicromtal")
+      backendCount <- backendCountFromConfig(config)
+      configuredSignificance = config.getOrElse("significance-level", 0.05)
+    } yield CromwellTracker(backendCount, configuredSignificance)
+  }
+  logger.info(s"Horicromtal tracker config: {}", cromwellTracker)
 }
 
 /**
@@ -51,6 +69,7 @@ trait CentaurTestSuiteShutdown extends Suite with BeforeAndAfterAll {
 
   override protected def afterAll() = {
     CromwellManager.stopCromwell("ScalaTest AfterAll")
+    CentaurTestSuite.cromwellTracker foreach { _.assertHoricromtality() }
     shutdownHook.foreach(_.remove())
   }
 }
@@ -59,6 +78,6 @@ trait CentaurTestSuiteShutdown extends Suite with BeforeAndAfterAll {
   * The main centaur test suites, runs sub suites in parallel, but allows better control over the way each nested suite runs.
   */
 class CentaurTestSuite
-  extends Suites(new SequentialTestCaseSpec(), new StandardTestCaseSpec())
+  extends Suites(new SequentialTestCaseSpec(), new ParallelTestCaseSpec())
     with ParallelTestExecution
     with CentaurTestSuiteShutdown

centaur/src/it/scala/centaur/ExternalTestCaseSpec.scala

@@ -16,7 +16,7 @@ class ExternalTestCaseSpec(cromwellBackends: List[String]) extends AbstractCenta
   }
 
   def runTestFile(testFile: String) = {
-    CentaurTestCase.fromFile(File(testFile)) match {
+    CentaurTestCase.fromFile(cromwellTracker = None)(File(testFile)) match {
       case Valid(testCase) => executeStandardTest(testCase)
       case Invalid(error) =>
         fail(s"Invalid test case: ${error.toList.mkString(", ")}")

centaur/src/it/scala/centaur/ParallelTestCaseSpec.scala

@@ -0,0 +1,18 @@
+package centaur
+
+import centaur.CentaurTestSuite.cromwellTracker
+import org.scalatest._
+
+
+/**
+  * Runs test cases in parallel, this should be the default type for tests unless they would otherwise crosstalk in undesirable
+  * ways with other tests and must be made sequential.
+  */
+@DoNotDiscover
+class ParallelTestCaseSpec(cromwellBackends: List[String])
+  extends AbstractCentaurTestCaseSpec(cromwellBackends, cromwellTracker = cromwellTracker) with ParallelTestExecution {
+
+  def this() = this(CentaurTestSuite.cromwellBackends)
+
+  allTestCases.filter(_.testFormat.isParallel) foreach executeStandardTest
+}

centaur/src/it/scala/centaur/SequentialTestCaseSpec.scala

@@ -10,7 +10,7 @@ import org.scalatest.{DoNotDiscover, Matchers}
 class SequentialTestCaseSpec(cromwellBackends: List[String]) extends AbstractCentaurTestCaseSpec(cromwellBackends) with Matchers {
 
   def this() = this(CentaurTestSuite.cromwellBackends)
-  
-  allTestCases.filter(CentaurTestSuite.runSequential) foreach executeStandardTest
+
+  allTestCases.filterNot(_.testFormat.isParallel) foreach executeStandardTest
 
 }

centaur/src/main/resources/integrationTestCases/Somatic/Mutect2/Mutect2.aws.inputs

@@ -0,0 +1,18 @@
+{
+    "Mutect2.preemptible_attempts": 0,
+    "Mutect2.normal_bam": "s3://cromwell-integration-tests/somatic-b37/HCC1143_normal.bam",
+    "Mutect2.tumor_bam": "s3://cromwell-integration-tests/somatic-b37/HCC1143.bam",
+    "Mutect2.normal_bai": "s3://cromwell-integration-tests/somatic-b37/HCC1143_normal.bai",
+    "Mutect2.run_oncotator": true,
+    "Mutect2.ref_fasta": "s3://cromwell-integration-tests/somatic-b37/Homo_sapiens_assembly19.fasta",
+    "Mutect2.gatk_docker": "us.gcr.io/broad-gatk/gatk:4.0.9.0",
+    "Mutect2.intervals": "s3://cromwell-integration-tests/somatic-b37/whole_exome_agilent_1.1_refseq_plus_3_boosters.Homo_sapiens_assembly19.baits.interval_list",
+    "Mutect2.tumor_bai": "s3://cromwell-integration-tests/somatic-b37/HCC1143.bai",
+    "Mutect2.oncotator_docker": "broadinstitute/oncotator:1.9.6.1",
+    "Mutect2.ref_fai": "s3://cromwell-integration-tests/somatic-b37/Homo_sapiens_assembly19.fasta.fai",
+    "Mutect2.default_config_file": "s3://cromwell-integration-tests/somatic-b37/onco_config.txt",
+    "Mutect2.ref_dict": "s3://cromwell-integration-tests/somatic-b37/Homo_sapiens_assembly19.dict",
+    "Mutect2.artifact_modes": ["G/T", "C/T"],
+    "Mutect2.onco_ds_tar_gz": "s3://cromwell-integration-tests/somatic-b37/oncotator_v1_ds_April052016.tar.gz",
+    "Mutect2.scatter_count": 50
+}

centaur/src/main/resources/integrationTestCases/mutect2.aws.test

@@ -0,0 +1,13 @@
+name: mutect2.aws
+testFormat: workflowsuccess
+backends: [ AWSBATCH ]
+
+files {
+  workflow: Somatic/Mutect2/Mutect2.wdl
+  inputs: Somatic/Mutect2/Mutect2.aws.inputs
+}
+
+metadata {
+  workflowName: Mutect2
+  status: Succeeded
+}

centaur/src/main/resources/standardTestCases/check_network_in_vpc.test

@@ -0,0 +1,14 @@
+name: check_network_in_vpc
+testFormat: workflowsuccess
+backends: [Papiv2-Virtual-Private-Cloud]
+
+files {
+  workflow: virtual_private_cloud/check_network_in_vpc.wdl
+}
+
+metadata {
+  workflowName: check_network_in_vpc
+  status: Succeeded
+
+  "outputs.check_network_in_vpc.network_used": "cromwell-ci-vpc-network"
+}

centaur/src/main/resources/standardTestCases/file_evaluator_identifier_lookups.test

@@ -0,0 +1,16 @@
+name: file_evaluator_identifier_lookups
+testFormat: workflowsuccess
+
+files {
+  workflow: file_evaluator_identifier_lookups/file_evaluator_identifier_lookups.wdl
+}
+
+metadata {
+  workflowName: file_evaluator_identifier_lookups
+  status: Succeeded
+  "outputs.file_evaluator_identifier_lookups.identifier.id": "OK"
+  "outputs.file_evaluator_identifier_lookups.identifier_member_access.left": "LEFT"
+  "outputs.file_evaluator_identifier_lookups.identifier_member_access.right": "RIGHT"
+  "outputs.file_evaluator_identifier_lookups.index_access.zero": "ZERO"
+  "outputs.file_evaluator_identifier_lookups.index_access.one": "ONE"
+}