Repository: odahu/odahu-flow (language: python)

Issue:
checks if pack integration exists when I hadn't passed it
**description**
if I pass an empty `integrationName` or don't specify it at all, then I get an error:
> Validation of model packaging is failed: entity "" is not found; you should specify artifactName; integration name must be nonempty

It shows that the `integration name` must be non-empty while trying to find packaging integration:
> entity "" is not found

**specification**
[packaging_no_required_params.txt](https://github.com/odahu/odahu-flow/files/5604709/packaging_no_required_params.txt)

**to reproduce**
1. run `odahuflowctl pack create -f packaging_no_required_params.txt`

**expected behavior**
should only show that `integration name must be nonempty`

**actual behavior**
shows that `integration name must be nonempty` and `entity "" is not found`


Discussion / hints:

Fix patch:
diff --git a/packages/operator/pkg/apiserver/routes/v1/packaging/model_packaging_validation.go b/packages/operator/pkg/apiserver/routes/v1/packaging/model_packaging_validation.go
index ff98375e4..9b1a90e3c 100644
--- a/packages/operator/pkg/apiserver/routes/v1/packaging/model_packaging_validation.go
+++ b/packages/operator/pkg/apiserver/routes/v1/packaging/model_packaging_validation.go
@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 	"github.com/odahu/odahu-flow/packages/operator/pkg/config"
+	odahu_errs "github.com/odahu/odahu-flow/packages/operator/pkg/errors"
 	"github.com/odahu/odahu-flow/packages/operator/pkg/validation"
 	"reflect"
 
@@ -65,32 +66,51 @@ func NewMpValidator(
 	}
 }
 
-func (mpv *MpValidator) ValidateAndSetDefaults(mp *packaging.ModelPackaging) (err error) {
-	err = multierr.Append(err, mpv.validateMainParameters(mp))
-
-	if len(mp.Spec.IntegrationName) == 0 {
-		err = multierr.Append(err, errors.New(EmptyIntegrationNameErrorMessage))
-	} else {
-		if pi, k8sErr := mpv.piRepo.GetPackagingIntegration(mp.Spec.IntegrationName); k8sErr != nil {
-			err = multierr.Append(err, k8sErr)
-		} else {
-			err = multierr.Append(err, mpv.validateArguments(pi, mp))
+// ValidateAndSetDefaultsPIRequired validates ModelPackaging fields using a corresponding PackagingIntegration
+func (mpv *MpValidator) ValidateAndSetDefaultsPIRequired(mp *packaging.ModelPackaging) (err error) {
 
-			err = multierr.Append(err, mpv.validateTargets(pi, mp))
+	pi, piErr := mpv.piRepo.GetPackagingIntegration(mp.Spec.IntegrationName)
+	switch {
+	case piErr == nil:
+		if len(mp.Spec.Image) == 0 {
+			mp.Spec.Image = pi.Spec.DefaultImage
+			logMP.Info("Model packaging id is empty. Set a packaging integration image",
+				"id", mp.ID, "image", mp.Spec.Image)
 		}
-
+		err = multierr.Append(err, mpv.validateArguments(pi, mp))
+		err = multierr.Append(err, mpv.validateTargets(pi, mp))
+	case odahu_errs.IsNotFoundError(piErr):
+		err = multierr.Append(err,
+			fmt.Errorf(
+				"packaging integration with name .Spec.IntegrationName = \"%s\" is not found",
+				mp.Spec.IntegrationName,
+			))
+	default:
+		// We are not able to complete validation in this case
+		panic(fmt.Errorf(
+			"unable to get Packaging Integration: %v", piErr,
+		))
 	}
 
-	err = multierr.Append(err, mpv.validateOutputConnection(mp))
+	return err
+}
 
-	err = multierr.Append(err, mpv.validateNodeSelector(mp))
+func (mpv *MpValidator) ValidateAndSetDefaults(mp *packaging.ModelPackaging) (err error) {
 
+
+	err = multierr.Append(err, mpv.validateMainParameters(mp))
+	err = multierr.Append(err, mpv.validateOutputConnection(mp))
+	err = multierr.Append(err, mpv.validateNodeSelector(mp))
 	err = multierr.Append(err, validation.ValidateResources(mp.Spec.Resources, config.NvidiaResourceName))
 
+	if len(mp.Spec.IntegrationName) == 0 {
+		err = multierr.Append(err, errors.New(EmptyIntegrationNameErrorMessage))
+	} else {
+		err = multierr.Append(err, mpv.ValidateAndSetDefaultsPIRequired(mp))
+	}
 	if err != nil {
 		return fmt.Errorf("%s: %s", ValidationMpErrorMessage, err.Error())
 	}
-
 	return nil
 }
 
@@ -107,16 +127,6 @@ func (mpv *MpValidator) validateMainParameters(mp *packaging.ModelPackaging) (er
 
 	err = multierr.Append(err, validation.ValidateID(mp.ID))
 
-	if len(mp.Spec.Image) == 0 {
-		packagingIntegration, k8sErr := mpv.piRepo.GetPackagingIntegration(mp.Spec.IntegrationName)
-		if k8sErr != nil {
-			err = multierr.Append(err, k8sErr)
-		} else {
-			mp.Spec.Image = packagingIntegration.Spec.DefaultImage
-			logMP.Info("Model packaging id is empty. Set a packaging integration image",
-				"id", mp.ID, "image", mp.Spec.Image)
-		}
-	}
 
 	if len(mp.Spec.ArtifactName) == 0 {
 		err = multierr.Append(err, errors.New(TrainingIDOrArtifactNameErrorMessage))


Test patch:
diff --git a/packages/operator/pkg/apiserver/routes/v1/packaging/model_packaging_validation_test.go b/packages/operator/pkg/apiserver/routes/v1/packaging/model_packaging_validation_test.go
index bdf17b43c..7f5f59eca 100644
--- a/packages/operator/pkg/apiserver/routes/v1/packaging/model_packaging_validation_test.go
+++ b/packages/operator/pkg/apiserver/routes/v1/packaging/model_packaging_validation_test.go
@@ -246,19 +246,6 @@ func (s *ModelPackagingValidationSuite) TestMpImageExplicitly() {
 	s.g.Expect(mp.Spec.Image).Should(Equal(image))
 }
 
-func (s *ModelPackagingValidationSuite) TestMpImageNotFound() {
-	mp := &packaging.ModelPackaging{
-		Spec: packaging.ModelPackagingSpec{
-			IntegrationName: "not found",
-		},
-	}
-
-	err := s.validator.ValidateAndSetDefaults(mp)
-	s.g.Expect(err).Should(HaveOccurred())
-	s.g.Expect(err.Error()).Should(ContainSubstring(
-		"entity \"not found\" is not found"))
-}
-
 func (s *ModelPackagingValidationSuite) TestMpArtifactName() {
 	mp := &packaging.ModelPackaging{
 		Spec: packaging.ModelPackagingSpec{
@@ -289,6 +276,10 @@ func (s *ModelPackagingValidationSuite) TestMpIntegrationNameEmpty() {
 	err := s.validator.ValidateAndSetDefaults(mp)
 	s.g.Expect(err).Should(HaveOccurred())
 	s.g.Expect(err.Error()).Should(ContainSubstring(pack_route.EmptyIntegrationNameErrorMessage))
+	// "not found" substring here is expected in case when packaging integration can't be found by
+	// .Spec.IntegrationName, but because .Spec.IntegrationName is empty we shouldn't try to find
+	// it at all
+	s.g.Expect(err.Error()).Should(Not(ContainSubstring("not found")))
 }
 
 func (s *ModelPackagingValidationSuite) TestMpIntegrationNotFound() {
@@ -301,7 +292,7 @@ func (s *ModelPackagingValidationSuite) TestMpIntegrationNotFound() {
 	err := s.validator.ValidateAndSetDefaults(mp)
 	s.g.Expect(err).Should(HaveOccurred())
 	s.g.Expect(err.Error()).Should(ContainSubstring(
-		"entity \"some-packaging-name\" is not found"))
+		"packaging integration with name .Spec.IntegrationName = \"some-packaging-name\" is not found"))
 }
 
 func (s *ModelPackagingValidationSuite) TestMpNotValidArgumentsSchema() {