fix: check plan.IsTerminated on IsPlanRunning (#432)

---------

Signed-off-by: ThibaultFy <thibault.fouqueray@gmail.com>

changes/432.changed

@@ -0,0 +1 @@
+Rename `ComputePlanDBAL().IsPlanRunning(key)` to `ComputePlanDBAL().ArePlanTaksRunning(key)`

changes/432.fixed

@@ -0,0 +1 @@
+`IsPlanRunning` checks if a cancel of failure date is set before checking the tasks status.

e2e/computeplan_test.go

@@ -703,10 +703,14 @@ func TestIsPlanRunning(t *testing.T) {
 	require.Nil(t, err)
 
 	resp = appClient.IsPlanRunning("cp2")
-	require.True(t, resp.IsRunning)
+	require.False(t, resp.IsRunning)
 
-	appClient.CancelTask("task-cp2")
+	appClient.RegisterComputePlan(client.DefaultComputePlanOptions().WithKeyRef("cp3"))
+	appClient.RegisterTasks(client.DefaultTrainTaskOptions().WithPlanRef("cp3").WithKeyRef("task-cp3"))
+	appClient.StartTask("task-cp3")
 
-	resp = appClient.IsPlanRunning("cp2")
+	appClient.CancelTask("task-cp3")
+
+	resp = appClient.IsPlanRunning("cp3")
 	require.False(t, resp.IsRunning)
 }

lib/persistence/computeplan_dbal.go

@@ -15,7 +15,7 @@ type ComputePlanDBAL interface {
 	SetComputePlanName(plan *asset.ComputePlan, name string) error
 	CancelComputePlan(plan *asset.ComputePlan, cancelationDate time.Time) error
 	FailComputePlan(plan *asset.ComputePlan, failureDate time.Time) error
-	IsPlanRunning(key string) (bool, error)
+	ArePlanTasksRunning(key string) (bool, error)
 }
 
 type ComputePlanDBALProvider interface {

lib/service/computeplan.go

@@ -190,5 +190,10 @@ func (s *ComputePlanService) computePlanExists(key string) (bool, error) {
 // IsPlanRunning indicates whether there are tasks belonging to the compute plan
 // being executed or waiting to be executed
 func (s *ComputePlanService) IsPlanRunning(key string) (bool, error) {
-	return s.GetComputePlanDBAL().IsPlanRunning(key)
+	plan, err := s.GetPlan(key)
+	if plan.IsTerminated() {
+		return false, err
+	} else {
+		return s.GetComputePlanDBAL().ArePlanTasksRunning(key)
+	}
 }

lib/service/computetaskstate.go

@@ -135,19 +135,7 @@ func (s *ComputeTaskService) applyTaskAction(task *asset.ComputeTask, action ass
 	case asset.ComputeTaskAction_TASK_ACTION_FAILED:
 		transition = transitionFailed
 	case asset.ComputeTaskAction_TASK_ACTION_DONE:
-		if task.ComputePlanKey != "" {
-			plan, err := s.GetComputePlanService().GetPlan(task.ComputePlanKey)
-			if err != nil {
-				return err
-			}
-			if plan.IsTerminated() {
-				transition = transitionCanceled
-			} else {
-				transition = transitionDone
-			}
-		} else {
-			transition = transitionDone
-		}
+		transition = transitionDone
 	case asset.ComputeTaskAction_TASK_ACTION_BUILD_STARTED:
 		transition = transitionBuilding
 	case asset.ComputeTaskAction_TASK_ACTION_BUILD_FINISHED:

server/standalone/dbal/computeplan.go

@@ -173,7 +173,7 @@ func (d *DBAL) FailComputePlan(plan *asset.ComputePlan, failureDate time.Time) e
 	return d.updateComputePlan(plan.Key, "failure_date", failureDate)
 }
 
-func (d *DBAL) IsPlanRunning(key string) (bool, error) {
+func (d *DBAL) ArePlanTasksRunning(key string) (bool, error) {
 	stmt := getStatementBuilder().
 		Select("status",
 			"COUNT(status)").

server/standalone/dbal/computeplan_test.go

@@ -167,7 +167,7 @@ func TestIsPlanRunning(t *testing.T) {
 
 	dbal := &DBAL{ctx: context.TODO(), tx: tx, channel: testChannel}
 
-	isRunning, err := dbal.IsPlanRunning(cpKey)
+	isRunning, err := dbal.ArePlanTasksRunning(cpKey)
 	assert.NoError(t, err)
 	assert.True(t, isRunning)