diff --git a/core/src/main/scala/cromwell/core/path/DefaultPathBuilderFactory.scala b/core/src/main/scala/cromwell/core/path/DefaultPathBuilderFactory.scala index 91f05b10ef7..20a2afdd5ce 100644 --- a/core/src/main/scala/cromwell/core/path/DefaultPathBuilderFactory.scala +++ b/core/src/main/scala/cromwell/core/path/DefaultPathBuilderFactory.scala @@ -2,6 +2,7 @@ package cromwell.core.path import akka.actor.ActorSystem import cromwell.core.WorkflowOptions +import cromwell.core.path.PathBuilderFactory.PriorityDefault import scala.concurrent.{ExecutionContext, Future} @@ -9,4 +10,6 @@ case object DefaultPathBuilderFactory extends PathBuilderFactory { override def withOptions(options: WorkflowOptions)(implicit actorSystem: ActorSystem, ec: ExecutionContext) = Future.successful(DefaultPathBuilder) val name = "local" val tuple = name -> this + + override def priority: Int = PriorityDefault } diff --git a/core/src/main/scala/cromwell/core/path/PathBuilderFactory.scala b/core/src/main/scala/cromwell/core/path/PathBuilderFactory.scala index 1f215ba2933..79adaae4d6b 100644 --- a/core/src/main/scala/cromwell/core/path/PathBuilderFactory.scala +++ b/core/src/main/scala/cromwell/core/path/PathBuilderFactory.scala @@ -5,6 +5,7 @@ import cromwell.core.{Dispatcher, WorkflowOptions} import cats.syntax.traverse._ import cats.instances.list._ import cats.instances.future._ +import cromwell.core.path.PathBuilderFactory.PriorityStandard import scala.concurrent.{ExecutionContext, Future} @@ -12,16 +13,13 @@ object PathBuilderFactory { // Given a list of factories, instantiates the corresponding path builders def instantiatePathBuilders(factories: List[PathBuilderFactory], workflowOptions: WorkflowOptions)(implicit as: ActorSystem): Future[List[PathBuilder]] = { implicit val ec: ExecutionContext = as.dispatchers.lookup(Dispatcher.IoDispatcher) - // The DefaultPathBuilderFactory always needs to be last. - // The reason is path builders are tried in order, and the default one is very generous in terms of paths it "thinks" it supports - // For instance, it will return a Path for a gcs url even though it doesn't really support it - val sortedFactories = factories.sortWith({ - case (_, DefaultPathBuilderFactory) => true - case (DefaultPathBuilderFactory, _) => false - case (a, b) => factories.indexOf(a) < factories.indexOf(b) - }) + val sortedFactories = factories.sortBy(_.priority) sortedFactories.traverse(_.withOptions(workflowOptions)) } + + val PriorityBlob = 100 // High priority to evaluate first, because blob files may inadvertently match other filesystems + val PriorityStandard = 1000 + val PriorityDefault = 10000 // "Default" is a fallback, evaluate last } /** @@ -29,4 +27,11 @@ object PathBuilderFactory { */ trait PathBuilderFactory { def withOptions(options: WorkflowOptions)(implicit as: ActorSystem, ec: ExecutionContext): Future[PathBuilder] + + /** + * Candidate filesystems are considered in a stable order, as some requests may match multiple filesystems. + * To customize this order, the priority of a filesystem may be adjusted. Lower number == higher priority. + * @return This filesystem's priority + */ + def priority: Int = PriorityStandard } diff --git a/engine/src/main/scala/cromwell/engine/EngineFilesystems.scala b/engine/src/main/scala/cromwell/engine/EngineFilesystems.scala index 296d227d39f..c648261a7f4 100644 --- a/engine/src/main/scala/cromwell/engine/EngineFilesystems.scala +++ b/engine/src/main/scala/cromwell/engine/EngineFilesystems.scala @@ -8,19 +8,23 @@ import cromwell.core.filesystem.CromwellFileSystems import cromwell.core.path.{DefaultPathBuilderFactory, PathBuilder, PathBuilderFactory} import net.ceedubs.ficus.Ficus._ +import scala.collection.immutable.SortedMap import scala.concurrent.Future object EngineFilesystems { private val config: Config = ConfigFactory.load - private val defaultFileSystemFactory: Map[String, PathBuilderFactory] = + private val defaultFileSystemFactory: SortedMap[String, PathBuilderFactory] = Option(DefaultPathBuilderFactory.tuple) .filter(_ => config.as[Boolean]("engine.filesystems.local.enabled")) - .toMap + .to(collection.immutable.SortedMap) - private val pathBuilderFactories: Map[String, PathBuilderFactory] = { - CromwellFileSystems.instance.factoriesFromConfig(config.as[Config]("engine")) - .unsafe("Failed to instantiate engine filesystem") ++ defaultFileSystemFactory + private val pathBuilderFactories: SortedMap[String, PathBuilderFactory] = { + // Unordered maps are a classical source of randomness injection into a system + ( + CromwellFileSystems.instance.factoriesFromConfig(config.as[Config]("engine")) + .unsafe("Failed to instantiate engine filesystem") ++ defaultFileSystemFactory + ).to(collection.immutable.SortedMap) } def configuredPathBuilderFactories: List[PathBuilderFactory] = pathBuilderFactories.values.toList diff --git a/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilderFactory.scala b/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilderFactory.scala index e6d269a07b0..1a9df21b0f2 100644 --- a/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilderFactory.scala +++ b/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilderFactory.scala @@ -4,6 +4,7 @@ import akka.actor.ActorSystem import com.typesafe.config.Config import cromwell.core.WorkflowOptions import cromwell.core.path.PathBuilderFactory +import cromwell.core.path.PathBuilderFactory.PriorityBlob import net.ceedubs.ficus.Ficus._ import scala.concurrent.{ExecutionContext, Future} @@ -33,4 +34,6 @@ final case class BlobPathBuilderFactory(globalConfig: Config, instanceConfig: Co new BlobPathBuilder(container, endpoint)(fsm) } } + + override def priority: Int = PriorityBlob }