-
Notifications
You must be signed in to change notification settings - Fork 360
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
WX-1810 WX-1830 n1/n2/n2d machine types, cpuPlatform on GCPBATCH #7518
Changes from 7 commits
c00423f
fa71f1f
f9fb66f
b24f569
c0b6456
e311588
7f5cd0e
da484c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,9 +6,9 @@ | |
N2CustomMachineType, | ||
N2DCustomMachineType | ||
} | ||
import cromwell.core.logging.JobLogger | ||
import eu.timepit.refined.api.Refined | ||
import eu.timepit.refined.numeric.Positive | ||
import org.slf4j.Logger | ||
import wdl4s.parser.MemoryUnit | ||
import wom.format.MemorySize | ||
|
||
|
@@ -17,16 +17,17 @@ | |
cpu: Int Refined Positive, | ||
cpuPlatformOption: Option[String], | ||
googleLegacyMachineSelection: Boolean, | ||
jobLogger: Logger | ||
jobLogger: JobLogger | ||
): String = | ||
if (googleLegacyMachineSelection) { | ||
s"predefined-$cpu-${memory.to(MemoryUnit.MB).amount.intValue()}" | ||
} else { | ||
// If someone requests Intel Cascade Lake as their CPU platform then switch the machine type to n2. | ||
// If someone requests Intel Cascade Lake or Intel Ice Lake as their CPU platform then switch the machine type to n2. | ||
// Similarly, CPU platform of AMD Rome corresponds to the machine type n2d. | ||
val customMachineType = | ||
cpuPlatformOption match { | ||
case Some(GcpBatchRuntimeAttributes.CpuPlatformIntelCascadeLakeValue) => N2CustomMachineType | ||
case Some(GcpBatchRuntimeAttributes.CpuPlatformIntelIceLakeValue) => N2CustomMachineType | ||
Check warning on line 30 in supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/util/GcpBatchMachineConstraints.scala Codecov / codecov/patchsupportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/util/GcpBatchMachineConstraints.scala#L30
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can cover this with tests at GcpBatchMachineConstraintsSpec |
||
case Some(GcpBatchRuntimeAttributes.CpuPlatformAMDRomeValue) => N2DCustomMachineType | ||
case _ => N1CustomMachineType | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using named arguments to avoid confusing cpuPlatform/machineType because both are strings.