-
Notifications
You must be signed in to change notification settings - Fork 351
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-1595 Remove genomics usage from GCP Batch #7411
Conversation
The Batch backend configuration depends on entries prefixed by "genomics" which is the wrong term, this entry has been renamed to "batch-api". Also, the usages for these configs were renamed to "batch". At last, there was an usage involving lifesciences which has been removed too. NOTE: This is a breaking change, existing projecs using batch backend will need to update their configuration.
// Credentials object for the Genomics API | ||
private lazy val genomicsCredentials: Future[Credentials] = gcpBatchConfiguration.batchAttributes.auths.genomics | ||
// Credentials object for the Batch API | ||
private lazy val batchCredentials: Future[Credentials] = gcpBatchConfiguration.batchAttributes.auths.batch | ||
.retryCredentials(workflowOptions, | ||
List( | ||
CloudLifeSciencesScopes.CLOUD_PLATFORM, |
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.
I have tried removing this (CloudLifeSciencesScopes.CLOUD_PLATFORM
) and GenomicsScopes.GENOMICS
but centaur tests failed.
Does anyone know whether we should use a different scope? I'd expect that we shouldn't need anything lifesciences/genomics related (see https://github.com/broadinstitute/cromwell/actions/runs/8815748415/job/24198334692).
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.
This could use a CHANGELOG.md
entry to inform users about the config change. Maybe also explain that Batch support is in Alpha, hence there might be breaking changes.
| genomics { | ||
| // A reference to an auth defined in the `google` stanza at the top. This auth is used to create | ||
| // Pipelines and manipulate auth JSONs. | ||
| batch { |
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.
I'd personally prefer we use batch
everywhere for consistency.
Seems easier to get right, especially when transitioning from a product we just called genomics
.
@aednichols the comments are resolved and tests are passing, thanks. |
The Batch backend configuration depends on entries prefixed by "genomics" which is the wrong term, this entry has been renamed to "batch-api".
Also, the usages for these configs were renamed to "batch".
At last, there was an usage involving lifesciences which has been removed too.
NOTE: This is a breaking change, existing projecs using batch backend will need to update their configuration.