AN-349 AN-358 Fix cost status #7676
Conversation
| @@ -347,7 +347,8 @@ class MetadataBuilderActorSpec | |||
| val subWorkflow1aId = WorkflowId(UUID.fromString("1a1a1a1a-f76d-4af3-b371-5ba580916729")) | |||
| val subWorkflow1bId = WorkflowId(UUID.fromString("1b1b1b1b-f76d-4af3-b371-5ba580916729")) | |||
|
|
|||
| val workflowState = WorkflowSucceeded | |||
| val workflowSucceededState = WorkflowSucceeded | |||
| val workflowRunningState = WorkflowRunning | |||
There was a problem hiding this comment.
Confirmed that this test fails on develop
|
|
||
| def isComplete = subWorkflowsMetadata.size == waitFor | ||
| final case class HasReceivedCostEventsData(target: ActorRef, |
There was a problem hiding this comment.
Previously we used HasReceivedEventsData as the data object when collecting per-subworkflow results for either plain metadata or cost. I'm splitting it into two classes here for the two different use cases, so we can include workflow status in the cost one.
Opinions welcome on the naming - I find HasReceivedEventsData to be an unclear name for that object, but didn't want to introduce churn by changing it, so I tried to find a sensible-seeming name for a new parent trait.
There was a problem hiding this comment.
Honestly, i find that leaving an inline comment before the parent trait and then before the class to be more useful than trying to name things perfectly
| @@ -491,7 +512,7 @@ class MetadataBuilderActor(readMetadataWorkerMaker: () => Props, | |||
| if (newData.isComplete) { | |||
| buildCostAndStop( | |||
| data.originalQuery.workflowId, | |||
| extractFromJsAs[JsString](js, "status").map(_.value).getOrElse(""), // should never be empty | |||
There was a problem hiding this comment.
Very happy to delete this terrible line of code that past me wrote.
| () | ||
| case GetCost(wfId) if wfId == subWorkflow1aId => | ||
| sender() ! CostResponse(subWorkflow1aId, workflowState, MetadataLookupResponse(sub1aQuery, sub1aEvents)) | ||
| sender() ! CostResponse(subWorkflow1aId, |
There was a problem hiding this comment.
Nothing to do really and naming things are hard anyways, but I had to reread this 3 times before I noticed the 1, vs 1a, vs 1b, vs 2 😓
|
|
||
| def isComplete = subWorkflowsMetadata.size == waitFor | ||
| final case class HasReceivedCostEventsData(target: ActorRef, |
There was a problem hiding this comment.
Honestly, i find that leaving an inline comment before the parent trait and then before the class to be more useful than trying to name things perfectly
Description
Fixes a bug that caused an incorrect workflow status to be returned in the
/costresponse when subworkflows were present. This happened because subworkflow status was leaking into the parent workflow response.Also fixes some build issues resulting from GHA runners upgrading to Ubuntu 24.
Release Notes Confirmation
CHANGELOG.mdCHANGELOG.mdin this PRCHANGELOG.mdbecause it doesn't impact community usersTerra Release Notes