-
Notifications
You must be signed in to change notification settings - Fork 0
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
TSPS-303 validate method config and update if needed #121
Changes from all commits
d9dd09a
fcac6f9
6e30645
f3c493b
2db4c0c
6a29da5
9dee0d0
78b311e
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 |
---|---|---|
|
@@ -3,14 +3,21 @@ | |
import bio.terra.pipelines.app.configuration.internal.ImputationConfiguration; | ||
import bio.terra.pipelines.common.utils.FlightUtils; | ||
import bio.terra.pipelines.common.utils.PipelinesEnum; | ||
import bio.terra.pipelines.db.entities.PipelineInputDefinition; | ||
import bio.terra.pipelines.db.entities.PipelineOutputDefinition; | ||
import bio.terra.pipelines.dependencies.rawls.RawlsService; | ||
import bio.terra.pipelines.dependencies.rawls.RawlsServiceApiException; | ||
import bio.terra.pipelines.dependencies.sam.SamService; | ||
import bio.terra.pipelines.dependencies.stairway.JobMapKeys; | ||
import bio.terra.pipelines.stairway.imputation.RunImputationJobFlightMapKeys; | ||
import bio.terra.rawls.model.MethodConfiguration; | ||
import bio.terra.rawls.model.SubmissionReport; | ||
import bio.terra.rawls.model.SubmissionRequest; | ||
import bio.terra.stairway.*; | ||
import com.fasterxml.jackson.core.type.TypeReference; | ||
import java.util.List; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
/** | ||
* This step submits a submission to cromwell using the rawls submission endpoint. It uses | ||
|
@@ -25,6 +32,8 @@ public class SubmitCromwellSubmissionStep implements Step { | |
private final RawlsService rawlsService; | ||
private final ImputationConfiguration imputationConfiguration; | ||
|
||
private final Logger logger = LoggerFactory.getLogger(SubmitCromwellSubmissionStep.class); | ||
|
||
public SubmitCromwellSubmissionStep( | ||
RawlsService rawlsService, | ||
SamService samService, | ||
|
@@ -46,8 +55,11 @@ public StepResult doStep(FlightContext flightContext) { | |
JobMapKeys.PIPELINE_NAME.getKeyName(), | ||
JobMapKeys.DESCRIPTION.getKeyName(), | ||
RunImputationJobFlightMapKeys.WDL_METHOD_NAME, | ||
RunImputationJobFlightMapKeys.WDL_METHOD_VERSION, | ||
RunImputationJobFlightMapKeys.CONTROL_WORKSPACE_BILLING_PROJECT, | ||
RunImputationJobFlightMapKeys.CONTROL_WORKSPACE_NAME); | ||
RunImputationJobFlightMapKeys.CONTROL_WORKSPACE_NAME, | ||
RunImputationJobFlightMapKeys.PIPELINE_INPUT_DEFINITIONS, | ||
RunImputationJobFlightMapKeys.PIPELINE_OUTPUT_DEFINITIONS); | ||
|
||
String controlWorkspaceName = | ||
inputParameters.get(RunImputationJobFlightMapKeys.CONTROL_WORKSPACE_NAME, String.class); | ||
|
@@ -59,10 +71,73 @@ public StepResult doStep(FlightContext flightContext) { | |
String description = inputParameters.get(JobMapKeys.DESCRIPTION.getKeyName(), String.class); | ||
String wdlMethodName = | ||
inputParameters.get(RunImputationJobFlightMapKeys.WDL_METHOD_NAME, String.class); | ||
String wdlMethodVersion = | ||
inputParameters.get(RunImputationJobFlightMapKeys.WDL_METHOD_VERSION, String.class); | ||
|
||
List<PipelineInputDefinition> inputDefinitions = | ||
inputParameters.get( | ||
RunImputationJobFlightMapKeys.PIPELINE_INPUT_DEFINITIONS, new TypeReference<>() {}); | ||
List<PipelineOutputDefinition> outputDefinitions = | ||
inputParameters.get( | ||
RunImputationJobFlightMapKeys.PIPELINE_OUTPUT_DEFINITIONS, new TypeReference<>() {}); | ||
|
||
// validate and extract parameters from working map | ||
FlightMap workingMap = flightContext.getWorkingMap(); | ||
|
||
MethodConfiguration methodConfiguration; | ||
try { | ||
// grab current method config and validate it | ||
methodConfiguration = | ||
rawlsService.getCurrentMethodConfigForMethod( | ||
samService.getTeaspoonsServiceAccountToken(), | ||
controlWorkspaceProject, | ||
controlWorkspaceName, | ||
wdlMethodName); | ||
} catch (RawlsServiceApiException e) { | ||
// if we fail to grab the method config then retry | ||
return new StepResult(StepStatus.STEP_RESULT_FAILURE_RETRY, e); | ||
} | ||
boolean validMethodConfig = | ||
rawlsService.validateMethodConfig( | ||
methodConfiguration, | ||
pipelineName.getValue(), | ||
wdlMethodName, | ||
inputDefinitions, | ||
outputDefinitions, | ||
wdlMethodVersion); | ||
|
||
// if not a valid method config, set the method config to what we think it should be. This | ||
// shouldn't happen | ||
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. shouldn't happen often, but it will happen once every time we change the wdlMethodVersion... unless we update it in Terra as well as in our service, which actually we could do. 🤷 it's nice to have the warning log here. wonder if we should set up a slack channel to capture warnings. 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. is that something we can do -- is that a sentry thingy? 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. yeah probably sentry |
||
if (!validMethodConfig) { | ||
logger.warn( | ||
"found method config that was not valid for billing project: {}, workspace: {}, method name: {}, methodConfigVersion: {}", | ||
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. nit: it's not necessarily "not valid" but it's not matching our expectations. super minor but to me "not valid" suggests there's an actual error in the setup 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. but to us there is an error in the setup no? |
||
controlWorkspaceProject, | ||
controlWorkspaceName, | ||
wdlMethodName, | ||
methodConfiguration.getMethodConfigVersion()); | ||
|
||
MethodConfiguration updatedMethodConfiguration = | ||
rawlsService.updateMethodConfigToBeValid( | ||
methodConfiguration, | ||
pipelineName.getValue(), | ||
wdlMethodName, | ||
inputDefinitions, | ||
outputDefinitions, | ||
wdlMethodVersion); | ||
try { | ||
// update method config version, inputs, and outputs | ||
rawlsService.setMethodConfigForMethod( | ||
samService.getTeaspoonsServiceAccountToken(), | ||
updatedMethodConfiguration, | ||
controlWorkspaceProject, | ||
controlWorkspaceName, | ||
wdlMethodName); | ||
} catch (RawlsServiceApiException e) { | ||
// if we fail to update the method config then retry | ||
return new StepResult(StepStatus.STEP_RESULT_FAILURE_RETRY, e); | ||
} | ||
} | ||
|
||
// create submission request | ||
SubmissionRequest submissionRequest = | ||
new SubmissionRequest() | ||
|
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.
also validate rootEntity (or whatever it's called) that defines which data table it should read from