Skip to content

Conversation

@THWiseman
Copy link
Contributor

Added support for two more CPU types. Recommended reading: machine types.

  • Ice Lake was very simple to support since it is a customizable N2 machine like Cascade Lake is, and therefore can use identical logic.
  • Sapphire Rapids was slightly more involved since it is not customizable like the other machine types are. Added PredefinedMachineType.scala to support this endeavor and leave room for future platforms.

I ran some workflows with my local cromwell and confirmed that the VMs created in Google Cloud are appropriately provisioned. A centaur test should also confirm that this is working as expected.

@THWiseman THWiseman requested a review from a team as a code owner November 8, 2023 21:34
Copy link
Collaborator

@aednichols aednichols left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Could use a CHANGELOG.md entry.

case cpu if cpu > 8 && cpu <= 22 => c3Standard_22
case cpu if cpu > 22 && cpu <= 44 => c3Standard_44
case cpu if cpu > 44 && cpu <= 88 => c3Standard_88
case cpu if cpu > 88 => c3Standard_176
Copy link
Collaborator

Choose a reason for hiding this comment

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

We check these case statements in order and will use the first one that matches, so in each one you can assume that all previous case checks didn't match. So if you want you can make this more like

case cpu if cpu <= 4 => c3Standard_4
case cpu if cpu <= 8 => c3Standard_8
case cpu if cpu <= 22 => c3Standard_22
...

@THWiseman THWiseman mentioned this pull request Nov 13, 2023
@THWiseman THWiseman marked this pull request as draft November 15, 2023 14:36
@THWiseman THWiseman closed this Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants