CORE-349: request estimated costs from Cromwell for all workflows #3201
Conversation
| Future.successful(WorkflowCostBreakdown(id, workflowCost, "USD", workflowStatus, Seq.empty)) | ||
| if (abortedMap.keySet.contains(id)) | ||
| Future(WorkflowCostBreakdown(id, workflowCost, "USD", WorkflowStatuses.Aborted.toString, Seq.empty)) | ||
| else Future(WorkflowCostBreakdown(id, workflowCost, "USD", workflowStatus, Seq.empty)) |
There was a problem hiding this comment.
the submission monitor now calls getCost() for all workflows instead of calling status() for non-capped workflows and getCost() for capped workflows. Thus, the abort-related logic in this mock's status() needed to be copied into this mock's getCost().
| @@ -100,7 +100,7 @@ class SubmissionMonitorSpec(_system: ActorSystem) | |||
| workflowsRecs.map { workflowRec => | |||
| scala.util.Success( | |||
| Option( | |||
| (workflowRec.copy(status = WorkflowStatuses.Succeeded.toString), | |||
| (workflowRec.copy(status = WorkflowStatuses.Succeeded.toString, cost = Option(BigDecimal.valueOf(0))), | |||
| Some(ExecutionServiceOutputs(workflowRec.externalId.get, Map("o1" -> Left(AttributeString("foo"))))) | |||
There was a problem hiding this comment.
all workflows now return an estimated cost, so test expectations needed updating
| (workflowRec.copy(cost = Option(BigDecimal.valueOf(0))), None) | ||
| ) | ||
| ) | ||
| } |
There was a problem hiding this comment.
previously, the monitor would return None because the workflow's status hadn't changed. Now, since the monitor compares both status and cost, and the cost has changed, it returns Some instead of None.
| @@ -26,6 +26,7 @@ trait ExecutionServiceCluster extends ErrorReportable { | |||
| userInfo: UserInfo | |||
| ): Future[(ExecutionServiceId, Seq[Either[ExecutionServiceStatus, ExecutionServiceFailure]])] | |||
|
|
|||
| // currently unused | |||
| def status(workflowRec: WorkflowRecord, userInfo: UserInfo): Future[ExecutionServiceStatus] | |||
There was a problem hiding this comment.
I chose not to delete the now-unused status() method, but I could be convinced it needs cleaning up
| executionServiceCluster.status(workflowRec, petUser).map { newStatus => | ||
| if (newStatus.status != workflowRec.status) Option(workflowRec.copy(status = newStatus.status)) | ||
| else None | ||
| } |
There was a problem hiding this comment.
collapsing these two cases into one is the core of this PR
Ticket: CORE-349
All workflows now request their estimated cost from Cromwell and display that cost in the UI. Before this PR, we only requested estimated cost for workflows if a per-workflow cost threshold was set for the submission.
PR checklist
model/, then you should publish a new officialrawls-modeland perform the corresponding dependency updates as specified in the README: