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

[WX-1792] Emit Cost Per Hour #7550

Merged
merged 51 commits into from
Oct 11, 2024
Merged

[WX-1792] Emit Cost Per Hour #7550

merged 51 commits into from
Oct 11, 2024

Conversation

THWiseman
Copy link
Contributor

@THWiseman THWiseman commented Sep 18, 2024

Description

Creates vmCostPerHour metadata records for PAPIv2beta workflow tasks. Currently this cost includes only RAM and CPU, not GPU, disks, etc.

Release Notes Confirmation

CHANGELOG.md

  • I updated CHANGELOG.md in this PR
  • I assert that this change shouldn't be included in CHANGELOG.md because it doesn't impact community users

Terra Release Notes

  • I added a suggested release notes entry in this Jira ticket
  • I assert that this change doesn't need Jira release notes because it doesn't impact Terra users

@jgainerdewar jgainerdewar marked this pull request as ready for review September 30, 2024 21:01
@jgainerdewar jgainerdewar requested a review from a team as a code owner September 30, 2024 21:01
Comment on lines +90 to +96
/*
The cost catalog is annoyingly inconsistent and unstructured in this area.
- For N1 machines, only predefined SKUs are included, and they have "Predefined" in their description strings.
We will eventually fall back to using these SKUs for custom machines, but accurately represent them as Predefined here.
- For non-N1 machines, both custom and predefined SKUs are included, custom ones include "Custom" in their description
strings and predefined SKUs are only identifiable by the absence of "Custom."
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is so annoying. Definitely appreciate the comment!

@jgainerdewar
Copy link
Collaborator

This remains ready for review, but I haven't been able to do the scale testing I planned because there's a change needed to get the auth to GCP working in Terra deployments. Planning to take care of that and do the scale testing before merging this.

@jgainerdewar jgainerdewar merged commit f0454b9 into develop Oct 11, 2024
33 of 34 checks passed
@jgainerdewar jgainerdewar deleted the WX-1792-emit-cost-per-hour branch October 11, 2024 14:08
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