Skip to content

Commit

Permalink
WX-1595 GCP Batch cleanup (#7428)
Browse files Browse the repository at this point in the history
- GcpBatchAsyncBackendJobExecutionActor -> pollBackOff had maxInterval value hardcoded instead of using the config entry.
- GcpBatchTestConfig was still referencing papi instead of batch.
- Rename PipelinesApiEmptyMountedDisk to BatchApiEmptyMountedDisk.
- GcpBatchAsyncBackendJobExecutionActorSpec was still referencing pipelines instead of batch.
- Localization was referencing papi instead of batch.
- RunnableUtils had unused definitions which are now deleted.
  • Loading branch information
AlexITC committed May 15, 2024
1 parent 7cacb8f commit 101dd32
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -987,7 +987,11 @@ class GcpBatchAsyncBackendJobExecutionActor(override val standardParams: Standar
Future.successful(handle)
}

override lazy val pollBackOff: SimpleExponentialBackoff = SimpleExponentialBackoff(5.second, 5.minutes, 1.1)
override lazy val pollBackOff: SimpleExponentialBackoff = SimpleExponentialBackoff(
initialInterval = 5.second,
maxInterval = batchAttributes.maxPollingInterval.seconds,
multiplier = 1.1
)

override lazy val executeOrRecoverBackOff: SimpleExponentialBackoff =
SimpleExponentialBackoff(initialInterval = 5.seconds, maxInterval = 20.seconds, multiplier = 1.1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ object GcpBatchAttachedDisk {
}
case MountedDiskPattern(mountPoint, sizeGb, diskType) =>
(sizeGbValidation(sizeGb), diskTypeValidation(diskType)) mapN { (s, dt) =>
PipelinesApiEmptyMountedDisk(dt, s, DefaultPathBuilder.get(mountPoint))
BatchApiEmptyMountedDisk(dt, s, DefaultPathBuilder.get(mountPoint))
}
case _ =>
s"Disk strings should be of the format 'local-disk SIZE TYPE' or '/mount/point SIZE TYPE' but got: '$s'".invalidNel
Expand Down Expand Up @@ -65,8 +65,7 @@ trait GcpBatchAttachedDisk {
def mountPoint: Path
}

case class PipelinesApiEmptyMountedDisk(diskType: DiskType, sizeGb: Int, mountPoint: Path)
extends GcpBatchAttachedDisk {
case class BatchApiEmptyMountedDisk(diskType: DiskType, sizeGb: Int, mountPoint: Path) extends GcpBatchAttachedDisk {
val name = s"d-${mountPoint.pathAsString.md5Sum}"

override def toString: String = s"$mountPoint $sizeGb ${diskType.diskTypeName}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ trait Localization {
List(localizeDrsLocalizationManifest, runDrsLocalization)
} else List[Runnable.Builder]()

// Any "classic" PAPI v2 one-at-a-time localizations for non-GCS inputs.
// Any "classic" Batch one-at-a-time localizations for non-GCS inputs.
val singletonLocalizations =
createParameters.inputOutputParameters.fileInputParameters.flatMap(_.toRunnables(volumes))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,6 @@ import org.apache.commons.text.StringEscapeUtils

object RunnableUtils {

/** Image to use for ssh access. */
val sshImage = "gcr.io/cloud-genomics-pipelines/tools"

/** Entry point on the ssh image. */
val sshEntryPoint = "ssh-server"

/** Port mappings for the ssh container. */
val sshPortMappings = Map("22" -> Int.box(22))

private val config = ConfigFactory.load().getConfig("google")

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ class GcpBatchAsyncBackendJobExecutionActorSpec

val drsReadInterpreter: DrsReadInterpreter = (_, _) =>
throw new UnsupportedOperationException(
"PipelinesApiAsyncBackendJobExecutionActorSpec doesn't need to use drs read interpreter."
"GcpBatchAsyncBackendJobExecutionActorSpec doesn't need to use drs read interpreter."
)

DrsPathBuilder(
Expand Down Expand Up @@ -1018,13 +1018,12 @@ class GcpBatchAsyncBackendJobExecutionActorSpec
"strs" -> WomArray(WomArrayType(WomStringType), Seq("A", "B", "C").map(WomString))
)

class TestPipelinesApiExpressionFunctions
extends BatchExpressionFunctions(TestableStandardExpressionFunctionsParams) {
class TestBatchApiExpressionFunctions extends BatchExpressionFunctions(TestableStandardExpressionFunctionsParams) {
override def writeFile(path: String, content: String): Future[WomSingleFile] =
Future.fromTry(Success(WomSingleFile(s"gs://some/path/file.txt")))
}

val functions = new TestPipelinesApiExpressionFunctions
val functions = new TestBatchApiExpressionFunctions
val jesBackend = makeJesActorRef(SampleWdl.ArrayIO, Map.empty, "serialize", inputs, functions).underlyingActor
val jobDescriptor = jesBackend.jobDescriptor
val jesInputs = jesBackend.generateInputs(jobDescriptor)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import scala.util.Failure
class GcpBatchAttachedDiskSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers with TryValues {
val validTable = Table(
("unparsed", "parsed"),
("/mnt 3 SSD", PipelinesApiEmptyMountedDisk(DiskType.SSD, 3, DefaultPathBuilder.get("/mnt"))),
("/mnt/my_path 10 HDD", PipelinesApiEmptyMountedDisk(DiskType.HDD, 10, DefaultPathBuilder.get("/mnt/my_path"))),
("/mnt 3 SSD", BatchApiEmptyMountedDisk(DiskType.SSD, 3, DefaultPathBuilder.get("/mnt"))),
("/mnt/my_path 10 HDD", BatchApiEmptyMountedDisk(DiskType.HDD, 10, DefaultPathBuilder.get("/mnt/my_path"))),
("local-disk 100 SSD", GcpBatchWorkingDisk(DiskType.SSD, 100)),
("local-disk 100 LOCAL", GcpBatchWorkingDisk(DiskType.LOCAL, 100))
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class GcpBatchConfigurationSpec
forAll(configs) { (backend, global) =>
an[Exception] shouldBe thrownBy {
val failingGoogleConf = GoogleConfiguration(global)
val failingAttributes = GcpBatchConfigurationAttributes(failingGoogleConf, backend, "papi")
val failingAttributes = GcpBatchConfigurationAttributes(failingGoogleConf, backend, "batch")
new GcpBatchConfiguration(BackendConfigurationDescriptor(backend, global), failingGoogleConf, failingAttributes)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ object GcpBatchTestConfig {
| default = "batch"
| providers {
| batch {
| actor-factory = "cromwell.backend.google.pipelines.batch.GcpBatchBackendLifecycleActorFactory"
| actor-factory = "cromwell.backend.google.batch.GcpBatchBackendLifecycleActorFactory"
| config {
| $BatchBackendConfigString
| }
Expand Down

0 comments on commit 101dd32

Please sign in to comment.