Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BT-710 Add configs for BlobPathBuilderFactory #6817

Merged
merged 11 commits into from
Aug 4, 2022
17 changes: 17 additions & 0 deletions core/src/main/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,15 @@ google {
# They can be enabled individually in the engine.filesystems stanza and in the config.filesystems stanza of backends
# There is a default built-in local filesystem that can also be referenced as "local" as well.
filesystems {
blob {
class = "cromwell.filesystems.blob.BlobPathBuilderFactory"
global {
class = "cromwell.filesystems.blob.BlobFileSystemConfig"
config {
workspace-manager-url = "{PLACEHOLDER}"
}
}
}
drs {
class = "cromwell.filesystems.drs.DrsPathBuilderFactory"
# Use to share a unique global object across all instances of the factory
Expand Down Expand Up @@ -433,9 +442,17 @@ engine {
http {
enabled: true
}
blob {
sas-token = "{PLACEHOLDER}"
store = "{PLACEHOLDER}"
endpoint = "{PLACEHOLDER}"
workspace-id = "{PLACEHOLDER}"
enabled: false
}
}
}


languages {
default: WDL
WDL {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ package cromwell.webservice.routes.wes
import akka.actor.Props
import akka.http.scaladsl.testkit.ScalatestRouteTest
import akka.util.Timeout
import cromwell.languages.config.{CromwellLanguages, LanguageConfiguration}
import cromwell.languages.config.CromwellLanguages
import cromwell.languages.config.LanguageConfiguration
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind backing out these changes since we didn't otherwise change the file?

import cromwell.webservice.routes.CromwellApiService
import cromwell.webservice.routes.CromwellApiServiceSpec.MockWorkflowStoreActor
import org.scalatest.flatspec.AsyncFlatSpec
Expand All @@ -24,7 +25,7 @@ class ServiceInfoSpec extends AsyncFlatSpec with ScalatestRouteTest with Matcher

val expectedResponse = WesStatusInfoResponse(Map("CWL" -> Set("v1.0"), "WDL" -> Set("draft-2", "1.0", "biscayne")),
List("1.0"),
Set("ftp", "s3", "drs", "gcs", "http"),
Set("ftp", "s3", "drs", "gcs", "http", "blob"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, does this indicate that we're reporting the blob filesystem to WES users as something they can use? I don't think we want to do that yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll look into whats happening here

Map("Cromwell" -> CromwellApiService.cromwellVersion),
List(),
Map(WesState.Running -> 5, WesState.Queued -> 3, WesState.Canceling -> 2),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,20 @@ import com.typesafe.config.Config
import cromwell.core.WorkflowOptions
import cromwell.core.path.PathBuilderFactory
import cromwell.filesystems.blob.BlobPathBuilder
import net.ceedubs.ficus.Ficus._

import scala.concurrent.ExecutionContext
import scala.concurrent.Future

final case class BlobPathBuilderFactory(globalConfig: Config, instanceConfig: Config) extends PathBuilderFactory {
final case class BlobFileSystemConfig(config: Config)
final case class BlobPathBuilderFactory(globalConfig: Config, instanceConfig: Config, singletonConfig: BlobFileSystemConfig) extends PathBuilderFactory {
val sasToken: String = instanceConfig.as[String]("sas-token")
val container: String = instanceConfig.as[String]("store")
val endpoint: String = instanceConfig.as[String]("endpoint")
val workspaceId: String = instanceConfig.as[String]("workspace-id")
val workspaceManagerURL = singletonConfig.config.as[String]("workspace-manager-url")

override def withOptions(options: WorkflowOptions)(implicit as: ActorSystem, ec: ExecutionContext): Future[BlobPathBuilder] = {
val sasToken: String = instanceConfig.getString("sasToken")
val container: String = instanceConfig.getString("store")
val endpoint: String = instanceConfig.getString("endpoint")
Future {
new BlobPathBuilder(new AzureSasCredential(sasToken), container, endpoint)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package cromwell.filesystems.blob

import com.typesafe.config.ConfigFactory
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers

class BlobPathBuilderFactorySpec extends AnyFlatSpec with Matchers {

it should "parse configs for a functioning factory" in {
val endpoint = BlobPathBuilderSpec.buildEndpoint("coaexternalstorage")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not check in this storage account name.

It might be worth taking another look at this test as part of our conversations about overall testing strategy, but I think it's fine to merge for now.

val store = "inputs"
val sasToken = "{SAS TOKEN HERE}"
val workspaceId = "mockWorkspaceId"
val workspaceManagerURL = "https://test.ws.org"
val instanceConfig = ConfigFactory.parseString(
s"""
|sas-token = "$sasToken"
|store = "$store"
|endpoint = "$endpoint"
|workspace-id = "$workspaceId"
""".stripMargin)
val singletonConfig = ConfigFactory.parseString(s"""workspace-manager-url = "$workspaceManagerURL" """)
val globalConfig = ConfigFactory.parseString("""""")
val factory = BlobPathBuilderFactory(globalConfig, instanceConfig, new BlobFileSystemConfig(singletonConfig))
factory.container should equal(store)
factory.endpoint should equal(endpoint)
factory.sasToken should equal(sasToken)
factory.workspaceId should equal(workspaceId)
factory.workspaceManagerURL should equal(workspaceManagerURL)
}
}