Ensure run completion events with no artifacts return an empty slice …

…rather than nil (#403) Co-authored-by: Ian <[email protected]>
sky-uk · Nov 28, 2024 · 5fd91a2 · 5fd91a2
1 parent d2513a9
commit 5fd91a2
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 13 deletions.
diff --git a/argo/common/run_completion.go b/argo/common/run_completion.go
@@ -27,7 +27,7 @@ type RunCompletionEvent struct {
 	RunName               *NamespacedName `json:"runName,omitempty"`
 	RunId                 string          `json:"runId"`
 	ServingModelArtifacts []Artifact      `json:"servingModelArtifacts"`
-	Artifacts             []Artifact      `json:"artifacts,omitempty"`
+	Artifacts             []Artifact      `json:"artifacts"`
 	Provider              string          `json:"provider"`
 }
 

diff --git a/controllers/webhook/runcompletion_event_trigger.go b/controllers/webhook/runcompletion_event_trigger.go
@@ -3,6 +3,7 @@ package webhook
 import (
 	"context"
 	"fmt"
+
 	"github.com/sky-uk/kfp-operator/argo/common"
 	"google.golang.org/grpc/credentials/insecure"
 
@@ -85,7 +86,7 @@ func RunCompletionEventToProto(event common.RunCompletionEvent) (*pb.RunCompleti
 }
 
 func artifactToProto(commonArtifacts []common.Artifact) []*pb.Artifact {
-	var pbArtifacts []*pb.Artifact
+	pbArtifacts := []*pb.Artifact{}
 	for _, commonArtifact := range commonArtifacts {
 		pbArtifacts = append(pbArtifacts, &pb.Artifact{
 			Location: commonArtifact.Location,

diff --git a/controllers/webhook/runcompletion_event_trigger_unit_test.go b/controllers/webhook/runcompletion_event_trigger_unit_test.go
@@ -5,6 +5,7 @@ package webhook
 import (
 	"context"
 	"errors"
+
 	"github.com/go-logr/logr"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -97,6 +98,8 @@ var _ = Context("EventDataToPbRunCompletion", func() {
 
 		It("returns no error when event data is converted to proto runcompletion event", func() {
 			protoRce, err := RunCompletionEventToProto(rce)
+			Expect(err).NotTo(HaveOccurred())
+
 			expectedArtifacts := []*pb.Artifact{
 				{
 					Name:     "some-artifact",
@@ -114,12 +117,27 @@ var _ = Context("EventDataToPbRunCompletion", func() {
 				Provider:              "some-provider",
 			}
 			Expect(protoRce).To(Equal(expectedResult))
+		})
+
+		It("returns empty slices when there are no artifacts", func() {
+			rceWithoutArtifacts := rce
+			rceWithoutArtifacts.Artifacts = []common.Artifact{}
+			rceWithoutArtifacts.ServingModelArtifacts = []common.Artifact{}
+
+			protoRce, err := RunCompletionEventToProto(rceWithoutArtifacts)
 			Expect(err).NotTo(HaveOccurred())
+
+			emptySliceOfArtifacts := []*pb.Artifact{}
+
+			Expect(protoRce.Artifacts).To(Equal(emptySliceOfArtifacts))
+			Expect(protoRce.ServingModelArtifacts).To(Equal(emptySliceOfArtifacts))
 		})
 
 		It("returns no error when event data with no RunName provided is converted to proto runcompletion event", func() {
 			rce.RunName = nil
 			protoRce, err := RunCompletionEventToProto(rce)
+			Expect(err).NotTo(HaveOccurred())
+
 			expectedArtifacts := []*pb.Artifact{
 				{
 					Name:     "some-artifact",
@@ -137,7 +155,6 @@ var _ = Context("EventDataToPbRunCompletion", func() {
 				Provider:              "some-provider",
 			}
 			Expect(protoRce).To(Equal(expectedResult))
-			Expect(err).NotTo(HaveOccurred())
 		})
 
 		It("returns error when namespaced name fields cannot be marshalled", func() {

diff --git a/triggers/run-completion-event-trigger/cmd/functional_test.go b/triggers/run-completion-event-trigger/cmd/functional_test.go
@@ -6,12 +6,13 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"github.com/sky-uk/kfp-operator/argo/common"
-	"github.com/sky-uk/kfp-operator/triggers/run-completion-event-trigger/internal/publisher"
 	"log"
 	"testing"
 	"time"
 
+	"github.com/sky-uk/kfp-operator/argo/common"
+	"github.com/sky-uk/kfp-operator/triggers/run-completion-event-trigger/internal/publisher"
+
 	"google.golang.org/grpc/credentials/insecure"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -83,6 +84,8 @@ var _ = Context("RunCompletionEventTriggerService", Ordered, func() {
 				},
 			}
 
+			emptyArtifacts := []*pb.Artifact{}
+
 			runCompletionEvent := &pb.RunCompletionEvent{
 				PipelineName:          "some-pipeline-name",
 				Provider:              "some-provider",
@@ -91,7 +94,7 @@ var _ = Context("RunCompletionEventTriggerService", Ordered, func() {
 				RunName:               "some-run-name",
 				Status:                pb.Status_SUCCEEDED,
 				ServingModelArtifacts: artifacts,
-				Artifacts:             artifacts,
+				Artifacts:             emptyArtifacts,
 			}
 
 			ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)

diff --git a/triggers/run-completion-event-trigger/internal/converters/converters.go b/triggers/run-completion-event-trigger/internal/converters/converters.go
@@ -28,14 +28,14 @@ func ProtoRunCompletionToCommon(protoRunCompletion *pb.RunCompletionEvent) (comm
 		RunConfigurationName:  &runConfigurationName,
 		RunName:               &runName,
 		RunId:                 protoRunCompletion.RunId,
-		ServingModelArtifacts: artifactsConverter(protoRunCompletion.ServingModelArtifacts),
-		Artifacts:             artifactsConverter(protoRunCompletion.Artifacts),
+		ServingModelArtifacts: protoToArtifacts(protoRunCompletion.ServingModelArtifacts),
+		Artifacts:             protoToArtifacts(protoRunCompletion.Artifacts),
 		Provider:              protoRunCompletion.Provider,
 	}, nil
 }
 
-func artifactsConverter(artifacts []*pb.Artifact) []common.Artifact {
-	var commonArtifacts []common.Artifact
+func protoToArtifacts(artifacts []*pb.Artifact) []common.Artifact {
+	commonArtifacts := []common.Artifact{}
 
 	for _, pbArtifact := range artifacts {
 		commonArtifact := common.Artifact{

diff --git a/triggers/run-completion-event-trigger/internal/converters/converters_unit_test.go b/triggers/run-completion-event-trigger/internal/converters/converters_unit_test.go
@@ -3,9 +3,10 @@
 package converters
 
 import (
+	"testing"
+
 	"github.com/sky-uk/kfp-operator/argo/common"
 	pb "github.com/sky-uk/kfp-operator/triggers/run-completion-event-trigger/proto"
-	"testing"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -67,7 +68,7 @@ var _ = Context("ProtoRunCompletionToCommon", func() {
 	})
 })
 
-var _ = Context("artifactsConverter", func() {
+var _ = Context("protoToArtifacts", func() {
 
 	When("given a list of proto `Artifacts`", func() {
 		It("returns a list of artifacts in the common struct", func() {
@@ -82,7 +83,7 @@ var _ = Context("artifactsConverter", func() {
 				},
 			}
 
-			Expect(artifactsConverter(artifacts)).To(Equal(
+			Expect(protoToArtifacts(artifacts)).To(Equal(
 				[]common.Artifact{
 					{
 						Location: "some-location",