Skip to content

Commit

Permalink
Merge pull request #3106 from broadinstitute/cjl_initial_work_dir_req…
Browse files Browse the repository at this point in the history
…uirement_6

IWDR: Allow Array[File] expressions as listings
  • Loading branch information
cjllanwarne committed Jan 9, 2018
2 parents d31000c + 877ce96 commit 0fb8584
Show file tree
Hide file tree
Showing 14 changed files with 117 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import cats.syntax.traverse._
import cats.syntax.validated._
import akka.actor.{Actor, ActorLogging, ActorRef}
import akka.event.LoggingReceive
import cats.data.Validated.Valid
import common.exception.MessageAggregation
import common.util.TryUtil
import common.validation.ErrorOr.ErrorOr
Expand Down Expand Up @@ -249,14 +248,15 @@ trait StandardAsyncExecutionActor extends AsyncBackendJobExecutionActor with Sta

val adHocFileCreationInputs = jobDescriptor.evaluatedTaskInputs.map { case (k,v) => k.localName.value -> v }

def validateAdHocFile(value: WomValue): ErrorOr[WomFile] = value match {
case f: WomFile => Valid(f)
def validateAdHocFile(value: WomValue): ErrorOr[List[WomFile]] = value match {
case f: WomFile => List(f).valid
case a: WomArray => a.value.toList.traverse(validateAdHocFile).map(_.flatten)
case other => s"Ad-hoc file creation expression invalidly created a ${other.womType.toDisplayString} result.".invalidNel
}

val adHocFileCreations: ErrorOr[List[WomFile]] = jobDescriptor.taskCall.callable.adHocFileCreation.toList.traverse {
_.evaluateValue(adHocFileCreationInputs, backendEngineFunctions).flatMap(validateAdHocFile)
}
}.map(_.flatten)

val adHocFileCreationSideEffectFiles: ErrorOr[List[CommandSetupSideEffectFile]] = adHocFileCreations map { _ map {
f => CommandSetupSideEffectFile(f, Option(adHocFileLocalization(f)))
Expand All @@ -274,7 +274,10 @@ trait StandardAsyncExecutionActor extends AsyncBackendJobExecutionActor with Sta
// TODO CWL: toTry.get here. Is throwing an exception the best way to indicate command generation failure?
((adHocFileCreationSideEffectFiles, instantiatedCommandValidation) mapN { (adHocFiles, command) =>
command.copy(createdFiles = command.createdFiles ++ adHocFiles)
}).toTry.get
}).toTry match {
case Success(ic) => ic
case Failure(e) => throw new Exception("Failed to evaluate ad hoc files", e)
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,11 @@ trait WriteFunctions extends IoFunctionSet with AsyncIoFunctions {
case true => Future.successful(WomSingleFile(file.pathAsString))
}
}

override def copyFile(pathFrom: String, targetName: String): Future[WomSingleFile] = {
val source = _writeDirectory.root / pathFrom
val destination = _writeDirectory / targetName

asyncIo.copyAsync(source, destination).as(WomSingleFile(destination.pathAsString))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,6 @@ class TestReadLikeFunctions(sizeResult: Try[Double]) extends IoFunctionSet {
override def glob(pattern: String): Future[Seq[String]] = ???

override def size(params: Seq[Try[WomValue]]): Future[WomFloat] = Future.fromTry(sizeResult.map(WomFloat.apply))

override def copyFile(pathFrom: String, targetName: String): Future[WomFile] = ???
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
cwlVersion: v1.0
$graph:
- id: iwdr_array_listing
class: CommandLineTool
baseCommand: ['ls']
requirements:
- class: DockerRequirement
dockerPull: "python:3.5.0"
- class: InitialWorkDirRequirement
listing: $(inputs.files)

stdout: file_list
inputs:
- id: files
type:
type: array
items: File
default: [ "/Users/chrisl/Downloads/cwl/allRequirements.txt" ]
arguments: [ "*.txt" ]
outputs:
- id: file_list
type: string
outputBinding:
glob: file_list
loadContents: true
outputEval: $(self[0].contents.trim())
2 changes: 2 additions & 0 deletions core/src/main/scala/cromwell/core/NoIoFunctionSet.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ case object NoIoFunctionSet extends IoFunctionSet {

override def writeFile(path: String, content: String): Future[WomFile] = Future.failed(new NotImplementedError("writeFile is not available here"))

override def copyFile(pathFrom: String, targetName: String): Future[WomFile] = throw new Exception("copyFile is not available here")

override def stdout(params: Seq[Try[WomValue]]): Try[WomFile] = Failure(new NotImplementedError("stdout is not available here"))

override def stderr(params: Seq[Try[WomValue]]): Try[WomFile] = Failure(new NotImplementedError("stderr is not available here"))
Expand Down
8 changes: 6 additions & 2 deletions cwl/src/main/scala/cwl/CommandLineTool.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import cwl.CommandLineTool._
import cwl.CwlType.CwlType
import cwl.CwlVersion._
import cwl.command.ParentName
import cwl.CwlAny.EnhancedCwlAny
import cwl.CwlAny.EnhancedJson
import cwl.requirement.RequirementToAttributeMap
import eu.timepit.refined.W
import shapeless.syntax.singleton._
Expand Down Expand Up @@ -137,7 +137,11 @@ case class CommandLineTool private(
case CommandInputParameter(id, _, _, _, _, _, _, Some(default), Some(tpe)) =>
val inputType = tpe.fold(MyriadInputTypeToWomType)
val inputName = FullyQualifiedName(id).id
InputDefinitionWithDefault(inputName, inputType, ValueAsAnExpression(inputType.coerceRawValue(default.stringRepresentation).get))
val defaultValue = default match {
case CwlAny.Json(jsonDefault) => ValueAsAnExpression(inputType.coerceRawValue(jsonDefault.sprayJsonRepresentation).get)
case CwlAny.File(fileDefault) => ValueAsAnExpression(inputType.coerceRawValue(fileDefault.path.get).get)
}
InputDefinitionWithDefault(inputName, inputType, defaultValue)
case CommandInputParameter(id, _, _, _, _, _, _, None, Some(tpe)) =>
val inputType = tpe.fold(MyriadInputTypeToWomType)
val inputName = FullyQualifiedName(id).id
Expand Down
36 changes: 29 additions & 7 deletions cwl/src/main/scala/cwl/CwlWomExpression.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package cwl
import cats.data.NonEmptyList
import cats.syntax.either._
import cats.syntax.validated._
import cats.syntax.traverse._
import cats.instances.list._
import common.validation.ErrorOr.{ErrorOr, ShortCircuitingFlatMap}
import common.validation.Validation._
import cwl.InitialWorkDirRequirement.IwdrListingArrayEntry
Expand Down Expand Up @@ -48,7 +50,7 @@ final case class InitialWorkDirFileGeneratorExpression(entry: IwdrListingArrayEn
override def cwlExpressionType: WomType = WomSingleFileType
override def sourceString: String = entry.toString

override def evaluateValue(inputValues: Map[String, WomValue], ioFunctionSet: IoFunctionSet): ErrorOr[WomFile] = {
override def evaluateValue(inputValues: Map[String, WomValue], ioFunctionSet: IoFunctionSet): ErrorOr[WomValue] = {
def mustBeString(womValue: WomValue): ErrorOr[String] = womValue match {
case WomString(s) => s.validNel
case other => WomStringType.coerceRawValue(other).map(_.asInstanceOf[WomString].value).toErrorOr
Expand All @@ -69,19 +71,39 @@ final case class InitialWorkDirFileGeneratorExpression(entry: IwdrListingArrayEn
} yield writtenFile

case IwdrListingArrayEntry.ExpressionDirent(Expression.ECMAScriptExpression(contentExpression), direntEntryName, _) =>
ExpressionEvaluator.evalExpression(contentExpression, ParameterContext(inputValues)).toErrorOr.flatMap {
// TODO CWL: Once files have "local paths", we will be able to specify a new local name based on direntEntryName if necessary.
case f: WomFile => f.validNel
val entryEvaluation: ErrorOr[WomValue] = ExpressionEvaluator.evalExpression(contentExpression, ParameterContext().addInputs(inputValues)).toErrorOr
entryEvaluation flatMap {
case f: WomFile =>
val entryName: ErrorOr[String] = direntEntryName match {
case Some(en) => evaluateEntryName(en)
case None => f.value.split('/').last.validNel
}
entryName flatMap { en => Try(Await.result(ioFunctionSet.copyFile(f.value, en), Duration.Inf)).toErrorOr }
case other => for {
coerced <- WomStringType.coerceRawValue(other).toErrorOr
contentString = coerced.asInstanceOf[WomString].value
// We force the entryname to be specified, and then evaluate it:
entryNameUnoptioned <- direntEntryName.toErrorOr("Invalid dirent: Entry was a string but no file name was supplied")
entryname <- evaluateEntryName(entryNameUnoptioned)
writtenFile <- Try(Await.result(ioFunctionSet.writeFile(entryname, contentString), Duration.Inf)).toErrorOr
} yield writtenFile}

case _ => ??? // TODO WOM and the rest....
} yield writtenFile
}
case IwdrListingArrayEntry.ECMAScriptExpression(expression) =>
// A single expression which must evaluate to an array of Files
val expressionEvaluation: ErrorOr[WomValue] = ExpressionEvaluator.evalExpression(expression, ParameterContext().addInputs(inputValues)).toErrorOr

expressionEvaluation flatMap {
case array: WomArray if WomArrayType(WomSingleFileType).coercionDefined(array) =>
val newFileArray: ErrorOr[List[WomFile]] = array.value.map(_.valueString).toList.traverse { source: String =>
val dest = source.split('/').last
Try(Await.result(ioFunctionSet.copyFile(source, dest), Duration.Inf)).toErrorOr
}
newFileArray map { nfa => WomArray(WomArrayType(WomSingleFileType), nfa) }

case other => s"InitialWorkDirRequirement listing expression must be Array[File] but got ${other.womType.toDisplayString}".invalidNel
}

case _ => ??? // TODO CWL and the rest....
}
}

Expand Down
16 changes: 16 additions & 0 deletions cwl/src/main/scala/cwl/InitialWorkDirRequirement.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package cwl

import cwl.ExpressionEvaluator.ECMAScriptExpression
import cwl.InitialWorkDirRequirement._
import eu.timepit.refined.W
import shapeless.{:+:, CNil, _}
Expand Down Expand Up @@ -58,6 +59,21 @@ object InitialWorkDirRequirement {
def unapply(e: IwdrListingArrayEntry): Option[(Expression, Option[StringOrExpression], Boolean)] =
e.select[ExpressionDirent].map(sd => (sd.entry, sd.entryname, sd.writableWithDefault))
}
object FilePath {
def unapply(e: IwdrListingArrayEntry): Option[String] = e.select[File].flatMap(file => file.location)
}
object String {
def unapply(e: IwdrListingArrayEntry): Option[String] = e.select[StringOrExpression].flatMap(_.select[String])
}
object ECMAScriptExpression {
def unapply(e: IwdrListingArrayEntry): Option[ECMAScriptExpression] = for {
soe <- e.select[StringOrExpression]
expr <- soe.select[Expression]
ecmaScriptExpression <- expr.select[ECMAScriptExpression]
} yield ecmaScriptExpression


}
}

object IwdrListingArray {
Expand Down
6 changes: 5 additions & 1 deletion cwl/src/main/scala/cwl/JsUtil.scala
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@ object JsUtil {
WomMap(womMapType, keys.map(key =>
WomString(key) -> fromJavascript(scriptObjectMirror.get(key))
).toMap)
case _ => throw new IllegalArgumentException(s"Unexpected value: $value")
case array: Array[Object] =>
val values = array map fromJavascript
WomArray(values)

case _ => throw new IllegalArgumentException(s"Unexpected ${value.getClass.getSimpleName} value: $value")
}
}
}
11 changes: 9 additions & 2 deletions cwl/src/main/scala/cwl/TypeAliases.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import cwl.CommandLineTool._
import shapeless.{:+:, CNil}
import cwl.CwlType.CwlType
import io.circe.Json
import spray.json.JsValue
import wom.types.WomType
import spray.json._

trait TypeAliases {

Expand Down Expand Up @@ -100,9 +102,14 @@ trait TypeAliases {
}

object CwlAny {
implicit class EnhancedCwlAny(val cwlAny: CwlAny) extends AnyVal {
def stringRepresentation: String = cwlAny.select[Json] map { json => json.asString.getOrElse(json.toString) } getOrElse "CwlAny (not Json)"

implicit class EnhancedJson(val json: Json) extends AnyVal {
// Yes, urgh! This is disgusting but until we rewrite the entire JS coercion set for circe...
def sprayJsonRepresentation: JsValue = json.noSpaces.parseJson
}

object Json { def unapply(cwlAny: CwlAny): Option[Json] = cwlAny.select[Json] }
object File { def unapply(cwlAny: CwlAny): Option[File] = cwlAny.select[File] }
}

object MyriadInputType {
Expand Down
1 change: 1 addition & 0 deletions cwl/src/test/scala/cwl/CommandOutputExpressionSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class CommandOutputExpressionSpec extends FlatSpec with Matchers {
new IoFunctionSet {
override def readFile(path: String, maxBytes: Option[Int] = None, failOnOverflow: Boolean = false) = Future.successful(data)
override def writeFile(path: String, content: String) = throw new Exception("writeFile should not be used in this test")
override def copyFile(pathFrom: String, pathTo: String): Future[WomFile] = throw new Exception("copyFile should not be used in this test")
override def stdout(params: Seq[Try[WomValue]]) = throw new Exception("stdout should not be used in this test")
override def stderr(params: Seq[Try[WomValue]]) = throw new Exception("stderr should not be used in this test")
override def glob(pattern: String): Future[Seq[String]] = throw new Exception("glob should not be used in this test")
Expand Down
6 changes: 4 additions & 2 deletions engine/src/main/scala/cromwell/engine/EngineIoFunctions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ class EngineIoFunctions(val pathBuilders: List[PathBuilder], override val asyncI
override def stderr(params: Seq[Try[WomValue]]): Try[WomFile] = fail("stderr")
override def glob(pattern: String): Future[Seq[String]] = throw new NotImplementedError(s"glob(path, pattern) not implemented yet")

// Cromwell does not support writing files from the engine.
override def writeFile(path: String, content: String): Future[WomFile] = Future.failed(new Exception("Can't write files"))
override def writeFile(path: String, content: String): Future[WomFile] =
Future.failed(new Exception("Cromwell does not support writing files from a workflow context"))
override def copyFile(pathFrom: String, targetName: String): Future[WomFile] =
Future.failed(new Exception("Cromwell does not support copying files from a workflow context"))
}
1 change: 1 addition & 0 deletions wom/src/main/scala/wom/expression/WomExpression.scala
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ final case class ValueAsAnExpression(value: WomValue) extends WomExpression {
trait IoFunctionSet {
def readFile(path: String, maxBytes: Option[Int], failOnOverflow: Boolean): Future[String]
def writeFile(path: String, content: String): Future[WomFile]
def copyFile(pathFrom: String, targetName: String): Future[WomFile]
def stdout(params: Seq[Try[WomValue]]): Try[WomFile]
def stderr(params: Seq[Try[WomValue]]): Try[WomFile]
def glob(pattern: String): Future[Seq[String]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ final case class PlaceholderWomExpression(inputs: Set[String], fixedWomType: Wom
case object PlaceholderIoFunctionSet extends IoFunctionSet {
override def readFile(path: String, maxBytes: Option[Int] = None, failOnOverflow: Boolean = false): Future[String] = ???
override def writeFile(path: String, content: String): Future[WomFile] = ???
override def copyFile(pathFrom: String, targetName: String): Future[WomFile] = ???
override def stdout(params: Seq[Try[WomValue]]) = ???
override def stderr(params: Seq[Try[WomValue]]) = ???
override def glob(pattern: String): Future[Seq[String]] = ???
Expand Down

0 comments on commit 0fb8584

Please sign in to comment.