From 4512436c392513e28f485dfc8b7c77c531a487f9 Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Sat, 10 Aug 2024 09:27:11 +0800 Subject: [PATCH 01/17] [propeller][admin] Compiler literal type error handling Signed-off-by: Future-Outlier --- .../manager/impl/validation/execution_validator.go | 12 ++++++++++++ .../manager/impl/validation/launch_plan_validator.go | 12 ++++++++++++ .../pkg/manager/impl/validation/signal_validator.go | 11 +++++++++++ .../pkg/compiler/transformers/k8s/inputs.go | 4 ++-- flytepropeller/pkg/controller/nodes/array/handler.go | 1 + .../nodes/catalog/datacatalog/transformer.go | 1 + 6 files changed, 39 insertions(+), 2 deletions(-) diff --git a/flyteadmin/pkg/manager/impl/validation/execution_validator.go b/flyteadmin/pkg/manager/impl/validation/execution_validator.go index b01e878b5b..30b12ede08 100644 --- a/flyteadmin/pkg/manager/impl/validation/execution_validator.go +++ b/flyteadmin/pkg/manager/impl/validation/execution_validator.go @@ -2,6 +2,7 @@ package validation import ( "context" + "fmt" "regexp" "strings" @@ -98,6 +99,17 @@ func CheckAndFetchInputsForExecution( executionInputMap[name] = expectedInput.GetDefault() } else { inputType := validators.LiteralTypeForLiteral(executionInputMap[name]) + if inputType == nil { + return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, + fmt.Sprintf( + "invalid %s input wrong type.\n"+ + "Expected %s, but got %s.\n"+ + "Suggested solution: Please update all of your Flyte images to the latest version and try"+ + " again.", + name, expectedInput.GetVar().GetType().String(), inputType, + ), + ) + } if !validators.AreTypesCastable(inputType, expectedInput.GetVar().GetType()) { return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "invalid %s input wrong type. Expected %s, but got %s", name, expectedInput.GetVar().GetType(), inputType) } diff --git a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go index 6a4a3b137f..0d91c0c76e 100644 --- a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go +++ b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go @@ -2,6 +2,7 @@ package validation import ( "context" + "fmt" "google.golang.org/grpc/codes" @@ -143,6 +144,17 @@ func checkAndFetchExpectedInputForLaunchPlan( return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "unexpected fixed_input %s", name) } inputType := validators.LiteralTypeForLiteral(fixedInput) + if inputType == nil { + return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, + fmt.Sprintf( + "invalid %s input wrong type.\n"+ + "Expected %s, but got %s.\n"+ + "Suggested solution: Please update all of your Flyte images to the latest version and try"+ + " again.", + name, value.GetType().String(), inputType, + ), + ) + } if !validators.AreTypesCastable(inputType, value.GetType()) { return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "invalid fixed_input wrong type %s, expected %v, got %v instead", name, value.GetType(), inputType) diff --git a/flyteadmin/pkg/manager/impl/validation/signal_validator.go b/flyteadmin/pkg/manager/impl/validation/signal_validator.go index 477d57443a..4482d45ca2 100644 --- a/flyteadmin/pkg/manager/impl/validation/signal_validator.go +++ b/flyteadmin/pkg/manager/impl/validation/signal_validator.go @@ -2,6 +2,7 @@ package validation import ( "context" + "fmt" "google.golang.org/grpc/codes" @@ -76,6 +77,16 @@ func ValidateSignalSetRequest(ctx context.Context, db repositoryInterfaces.Repos if err != nil { return err } + if valueType == nil { + return errors.NewFlyteAdminErrorf(codes.InvalidArgument, + fmt.Sprintf( + "Invalid signal value for '%s'.\n"+ + "Expected type: %s, but received: %s.\n"+ + "Suggested solution: Ensure all Flyte images are updated to the latest version and try again.", + request.Value, lookupSignal.GetType().String(), valueType, + ), + ) + } if !propellervalidators.AreTypesCastable(lookupSignal.Type, valueType) { return errors.NewFlyteAdminErrorf(codes.InvalidArgument, "requested signal value [%v] is not castable to existing signal type [%v]", diff --git a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go index 2d967c560e..865cd84c7e 100644 --- a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go +++ b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go @@ -1,12 +1,11 @@ package k8s import ( - "k8s.io/apimachinery/pkg/util/sets" - "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core" "github.com/flyteorg/flyte/flytepropeller/pkg/compiler/common" "github.com/flyteorg/flyte/flytepropeller/pkg/compiler/errors" "github.com/flyteorg/flyte/flytepropeller/pkg/compiler/validators" + "k8s.io/apimachinery/pkg/util/sets" ) func validateInputs(nodeID common.NodeID, iface *core.TypedInterface, inputs core.LiteralMap, errs errors.CompileErrors) (ok bool) { @@ -36,6 +35,7 @@ func validateInputs(nodeID common.NodeID, iface *core.TypedInterface, inputs cor } inputType := validators.LiteralTypeForLiteral(inputVal) + // todo: add a new error for idl type not found if !validators.AreTypesCastable(inputType, v.Type) { errs.Collect(errors.NewMismatchingTypesErr(nodeID, inputVar, v.Type.String(), inputType.String())) continue diff --git a/flytepropeller/pkg/controller/nodes/array/handler.go b/flytepropeller/pkg/controller/nodes/array/handler.go index 315041cb51..45f6c29c13 100644 --- a/flytepropeller/pkg/controller/nodes/array/handler.go +++ b/flytepropeller/pkg/controller/nodes/array/handler.go @@ -186,6 +186,7 @@ func (a *arrayNodeHandler) Handle(ctx context.Context, nCtx interfaces.NodeExecu size := -1 for _, variable := range literalMap.Literals { literalType := validators.LiteralTypeForLiteral(variable) + // TODO: Add a new error for idl type not found switch literalType.Type.(type) { case *idlcore.LiteralType_CollectionType: collectionLength := len(variable.GetCollection().Literals) diff --git a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go index 5c0ac0c30b..0b5dcfdd96 100644 --- a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go +++ b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go @@ -54,6 +54,7 @@ func GenerateTaskOutputsFromArtifact(id core.Identifier, taskInterface core.Type expectedVarType := outputVariables[artifactData.Name].GetType() inputType := validators.LiteralTypeForLiteral(artifactData.Value) + // TODO: add a new error for idl type not found if !validators.AreTypesCastable(inputType, expectedVarType) { return nil, fmt.Errorf("unexpected artifactData: [%v] type: [%v] does not match any task output type: [%v]", artifactData.Name, inputType, expectedVarType) } From e28d5957ebb7b0336e2ec4f2e917deca8bfac5ef Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Thu, 29 Aug 2024 21:03:33 +0800 Subject: [PATCH 02/17] support all err msg Signed-off-by: Future-Outlier --- flytepropeller/pkg/compiler/errors/compiler_errors.go | 11 +++++++++++ .../pkg/compiler/transformers/k8s/inputs.go | 8 ++++++-- flytepropeller/pkg/controller/nodes/array/handler.go | 6 +++++- .../nodes/catalog/datacatalog/transformer.go | 4 +++- flytepropeller/pkg/controller/nodes/errors/codes.go | 1 + 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/flytepropeller/pkg/compiler/errors/compiler_errors.go b/flytepropeller/pkg/compiler/errors/compiler_errors.go index 2e0e15367a..5b6363e6b2 100755 --- a/flytepropeller/pkg/compiler/errors/compiler_errors.go +++ b/flytepropeller/pkg/compiler/errors/compiler_errors.go @@ -96,6 +96,9 @@ const ( // Field not found in the dataclass FieldNotFoundError ErrorCode = "FieldNotFound" + + // IDL not found when variable binding + IDLNotFoundError ErrorCode = "IDLTypeNotFound" ) func NewBranchNodeNotSpecified(branchNodeID string) *CompileError { @@ -211,6 +214,14 @@ func NewMismatchingTypesErr(nodeID, fromVar, fromType, toType string) *CompileEr ) } +func NewIDLTypeNotFoundErr(nodeID string) *CompileError { + return newError( + IDLNotFoundError, + "Input is an invalid type, please update all of your Flyte images to the latest version and try again.", + nodeID, + ) +} + func NewMismatchingBindingsErr(nodeID, sinkParam, expectedType, receivedType string) *CompileError { return newError( MismatchingBindings, diff --git a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go index 865cd84c7e..3360933ccf 100644 --- a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go +++ b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go @@ -1,11 +1,12 @@ package k8s import ( + "k8s.io/apimachinery/pkg/util/sets" + "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core" "github.com/flyteorg/flyte/flytepropeller/pkg/compiler/common" "github.com/flyteorg/flyte/flytepropeller/pkg/compiler/errors" "github.com/flyteorg/flyte/flytepropeller/pkg/compiler/validators" - "k8s.io/apimachinery/pkg/util/sets" ) func validateInputs(nodeID common.NodeID, iface *core.TypedInterface, inputs core.LiteralMap, errs errors.CompileErrors) (ok bool) { @@ -35,7 +36,10 @@ func validateInputs(nodeID common.NodeID, iface *core.TypedInterface, inputs cor } inputType := validators.LiteralTypeForLiteral(inputVal) - // todo: add a new error for idl type not found + if inputType == nil { + errs.Collect(errors.NewIDLTypeNotFoundErr(nodeID)) + continue + } if !validators.AreTypesCastable(inputType, v.Type) { errs.Collect(errors.NewMismatchingTypesErr(nodeID, inputVar, v.Type.String(), inputType.String())) continue diff --git a/flytepropeller/pkg/controller/nodes/array/handler.go b/flytepropeller/pkg/controller/nodes/array/handler.go index 45f6c29c13..cdb2f7e66f 100644 --- a/flytepropeller/pkg/controller/nodes/array/handler.go +++ b/flytepropeller/pkg/controller/nodes/array/handler.go @@ -186,7 +186,11 @@ func (a *arrayNodeHandler) Handle(ctx context.Context, nCtx interfaces.NodeExecu size := -1 for _, variable := range literalMap.Literals { literalType := validators.LiteralTypeForLiteral(variable) - // TODO: Add a new error for idl type not found + if literalType == nil { + return handler.DoTransition(handler.TransitionTypeEphemeral, + handler.PhaseInfoFailure(idlcore.ExecutionError_USER, errors.IDLNotFoundErr, "Input is an invalid type, please update all of your Flyte images to the latest version and try again.", nil), + ), nil + } switch literalType.Type.(type) { case *idlcore.LiteralType_CollectionType: collectionLength := len(variable.GetCollection().Literals) diff --git a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go index 0b5dcfdd96..980d922f78 100644 --- a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go +++ b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go @@ -54,7 +54,9 @@ func GenerateTaskOutputsFromArtifact(id core.Identifier, taskInterface core.Type expectedVarType := outputVariables[artifactData.Name].GetType() inputType := validators.LiteralTypeForLiteral(artifactData.Value) - // TODO: add a new error for idl type not found + if inputType == nil { + return nil, fmt.Errorf("please update all of your Flyte images to the latest version and try again") + } if !validators.AreTypesCastable(inputType, expectedVarType) { return nil, fmt.Errorf("unexpected artifactData: [%v] type: [%v] does not match any task output type: [%v]", artifactData.Name, inputType, expectedVarType) } diff --git a/flytepropeller/pkg/controller/nodes/errors/codes.go b/flytepropeller/pkg/controller/nodes/errors/codes.go index 53d3bbc8d7..dafccdc5c0 100644 --- a/flytepropeller/pkg/controller/nodes/errors/codes.go +++ b/flytepropeller/pkg/controller/nodes/errors/codes.go @@ -27,4 +27,5 @@ const ( CatalogCallFailed ErrorCode = "CatalogCallFailed" InvalidArrayLength ErrorCode = "InvalidArrayLength" PromiseAttributeResolveError ErrorCode = "PromiseAttributeResolveError" + IDLNotFoundErr ErrorCode = "IDLNotFoundErr" ) From 096a59a631d03db2e2105109635db3065387191c Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Fri, 30 Aug 2024 11:13:16 +0800 Subject: [PATCH 03/17] add execution_validator tests Signed-off-by: Future-Outlier --- .../validation/execution_validator_test.go | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go b/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go index 0040ee2759..fa7807883d 100644 --- a/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go +++ b/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go @@ -175,6 +175,43 @@ func TestValidateExecEmptyInputs(t *testing.T) { assert.EqualValues(t, expectedMap, *actualInputs) } +func TestValidateExecUnknownIDLInputs(t *testing.T) { + unsupportedLiteral := &core.Literal{ + Value: &core.Literal_Scalar{ + Scalar: &core.Scalar{}, + }, + } + defaultInputs := &core.ParameterMap{ + Parameters: map[string]*core.Parameter{ + "foo": { + Var: &core.Variable{ + // 1000 means an unsupported type + Type: &core.LiteralType{Type: &core.LiteralType_Simple{Simple: 1000}}, + }, + Behavior: &core.Parameter_Default{ + Default: unsupportedLiteral, + }, + }, + }, + } + userInputs := &core.LiteralMap{ + Literals: map[string]*core.Literal{ + "foo": unsupportedLiteral, // This will lead to a nil inputType + }, + } + + _, err := CheckAndFetchInputsForExecution( + userInputs, + nil, + defaultInputs, + ) + assert.NotNil(t, err) + + expectedErrorMsg := "invalid foo input wrong type.\nExpected simple:1000, but got .\nSuggested solution: Please update all of your Flyte images to the latest version and try again." + assert.Equal(t, expectedErrorMsg, err.Error()) + +} + func TestValidExecutionId(t *testing.T) { err := CheckValidExecutionID("abcde123", "a") assert.Nil(t, err) From 07c9eb2c5235b474687a870580e547ccecd580d0 Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Fri, 30 Aug 2024 11:41:18 +0800 Subject: [PATCH 04/17] add launch_plan_validator tests Signed-off-by: Future-Outlier --- .../validation/execution_validator_test.go | 1 - .../validation/launch_plan_validator_test.go | 44 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go b/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go index fa7807883d..4babb6cfdd 100644 --- a/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go +++ b/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go @@ -209,7 +209,6 @@ func TestValidateExecUnknownIDLInputs(t *testing.T) { expectedErrorMsg := "invalid foo input wrong type.\nExpected simple:1000, but got .\nSuggested solution: Please update all of your Flyte images to the latest version and try again." assert.Equal(t, expectedErrorMsg, err.Error()) - } func TestValidExecutionId(t *testing.T) { diff --git a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go index 178c2b497b..1c5abd1ac6 100644 --- a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go +++ b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go @@ -231,6 +231,50 @@ func TestGetLpExpectedInvalidFixedInput(t *testing.T) { assert.Nil(t, actualMap) } +func TestGetLpExpectedInvalidFixedInputWithUnknownIDL(t *testing.T) { + unsupportedLiteral := &core.Literal{ + Value: &core.Literal_Scalar{ + Scalar: &core.Scalar{}, + }, + } + workflowVariableMap := &core.VariableMap{ + Variables: map[string]*core.Variable{ + "foo": { + Type: &core.LiteralType{Type: &core.LiteralType_Simple{Simple: 1000}}, + }, + }, + } + defaultInputs := &core.ParameterMap{ + Parameters: map[string]*core.Parameter{ + "foo": { + Var: &core.Variable{ + // 1000 means an unsupported type + Type: &core.LiteralType{Type: &core.LiteralType_Simple{Simple: 1000}}, + }, + Behavior: &core.Parameter_Default{ + Default: unsupportedLiteral, + }, + }, + }, + } + fixedInputs := &core.LiteralMap{ + Literals: map[string]*core.Literal{ + "foo": unsupportedLiteral, // This will lead to a nil inputType + }, + } + + _, err := checkAndFetchExpectedInputForLaunchPlan( + workflowVariableMap, + fixedInputs, + defaultInputs, + ) + + assert.NotNil(t, err) + + expectedErrorMsg := "invalid foo input wrong type.\nExpected simple:1000, but got .\nSuggested solution: Please update all of your Flyte images to the latest version and try again." + assert.Equal(t, expectedErrorMsg, err.Error()) +} + func TestGetLpExpectedNoFixedInput(t *testing.T) { request := testutils.GetLaunchPlanRequest() actualMap, err := checkAndFetchExpectedInputForLaunchPlan( From 426716050e1ebbd7ecaa3bdc338669e7281f128b Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Fri, 30 Aug 2024 15:18:36 +0800 Subject: [PATCH 05/17] add tests for signal_validator Signed-off-by: Future-Outlier --- .../impl/validation/signal_validator_test.go | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/flyteadmin/pkg/manager/impl/validation/signal_validator_test.go b/flyteadmin/pkg/manager/impl/validation/signal_validator_test.go index 9085c87ba7..4d8cf78927 100644 --- a/flyteadmin/pkg/manager/impl/validation/signal_validator_test.go +++ b/flyteadmin/pkg/manager/impl/validation/signal_validator_test.go @@ -283,4 +283,56 @@ func TestValidateSignalUpdateRequest(t *testing.T) { utils.AssertEqualWithSanitizedRegex(t, "requested signal value [scalar:{ primitive:{ boolean:false } } ] is not castable to existing signal type [[8 1]]", ValidateSignalSetRequest(ctx, repo, request).Error()) }) + + t.Run("UnknownIDLType", func(t *testing.T) { + ctx := context.TODO() + + // Define an unsupported literal type with a simple type of 1000 + unsupportedLiteralType := &core.LiteralType{ + Type: &core.LiteralType_Simple{ + Simple: 1000, // Using 1000 as an unsupported type + }, + } + unsupportedLiteralTypeBytes, _ := proto.Marshal(unsupportedLiteralType) + + // Mock the repository to return a signal with this unsupported type + repo := repositoryMocks.NewMockRepository() + repo.SignalRepo().(*repositoryMocks.SignalRepoInterface). + OnGetMatch(mock.Anything, mock.Anything).Return( + models.Signal{ + Type: unsupportedLiteralTypeBytes, // Set the unsupported type + }, + nil, + ) + + // Set up the unsupported literal that will trigger the nil valueType condition + unsupportedLiteral := &core.Literal{ + Value: &core.Literal_Scalar{ + Scalar: &core.Scalar{}, + }, + } + + request := admin.SignalSetRequest{ + Id: &core.SignalIdentifier{ + ExecutionId: &core.WorkflowExecutionIdentifier{ + Project: "project", + Domain: "domain", + Name: "name", + }, + SignalId: "signal", + }, + Value: unsupportedLiteral, // This will lead to valueType being nil + } + + // Invoke the function and check for the expected error + err := ValidateSignalSetRequest(ctx, repo, request) + assert.NotNil(t, err) + + // Expected error message + expectedErrorMsg := "Invalid signal value for 'scalar:{}'.\n" + + "Expected type: simple:1000, but received: .\n" + + "Suggested solution: Ensure all Flyte images are updated to the latest version and try again." + assert.Equal(t, expectedErrorMsg, err.Error()) + + }) } From d4a0dfef1238355f5f600f33ef8a0bb7aaa9bbe2 Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Fri, 30 Aug 2024 15:39:44 +0800 Subject: [PATCH 06/17] change var Signed-off-by: Future-Outlier --- flytepropeller/pkg/compiler/errors/compiler_error_test.go | 1 + flytepropeller/pkg/compiler/errors/compiler_errors.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/flytepropeller/pkg/compiler/errors/compiler_error_test.go b/flytepropeller/pkg/compiler/errors/compiler_error_test.go index 5a993aeff2..f5c89e2704 100644 --- a/flytepropeller/pkg/compiler/errors/compiler_error_test.go +++ b/flytepropeller/pkg/compiler/errors/compiler_error_test.go @@ -33,6 +33,7 @@ func TestErrorCodes(t *testing.T) { UnrecognizedValue: NewUnrecognizedValueErr("", ""), WorkflowBuildError: NewWorkflowBuildError(errors.New("")), NoNodesFound: NewNoNodesFoundErr(""), + IDLTypeNotFoundError: NewIDLTypeNotFoundErr(""), } for key, value := range testCases { diff --git a/flytepropeller/pkg/compiler/errors/compiler_errors.go b/flytepropeller/pkg/compiler/errors/compiler_errors.go index 5d898c1703..16b55249a7 100755 --- a/flytepropeller/pkg/compiler/errors/compiler_errors.go +++ b/flytepropeller/pkg/compiler/errors/compiler_errors.go @@ -98,7 +98,7 @@ const ( FieldNotFoundError ErrorCode = "FieldNotFound" // IDL not found when variable binding - IDLNotFoundError ErrorCode = "IDLTypeNotFound" + IDLTypeNotFoundError ErrorCode = "IDLTypeNotFound" ) func NewBranchNodeNotSpecified(branchNodeID string) *CompileError { @@ -223,7 +223,7 @@ func NewMismatchingVariablesErr(nodeID, fromVar, fromType, toVar, toType string) func NewIDLTypeNotFoundErr(nodeID string) *CompileError { return newError( - IDLNotFoundError, + IDLTypeNotFoundError, "Input is an invalid type, please update all of your Flyte images to the latest version and try again.", nodeID, ) From e9d2717aa21f32609f613a810939490a71094dab Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Fri, 30 Aug 2024 15:56:38 +0800 Subject: [PATCH 07/17] add tests for compiler_errors Signed-off-by: Future-Outlier --- flytepropeller/pkg/compiler/errors/compiler_error_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flytepropeller/pkg/compiler/errors/compiler_error_test.go b/flytepropeller/pkg/compiler/errors/compiler_error_test.go index f5c89e2704..aee616ad88 100644 --- a/flytepropeller/pkg/compiler/errors/compiler_error_test.go +++ b/flytepropeller/pkg/compiler/errors/compiler_error_test.go @@ -49,6 +49,6 @@ func TestIncludeSource(t *testing.T) { SetConfig(Config{IncludeSource: true}) e = NewCycleDetectedInWorkflowErr("", "") - assert.Equal(t, e.source, "compiler_error_test.go:50") + assert.Equal(t, e.source, "compiler_error_test.go:51") SetConfig(Config{}) } From 92c54b1ed45098b9085ccc1a48638bc71fd6f65c Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Fri, 30 Aug 2024 16:39:13 +0800 Subject: [PATCH 08/17] support nodes/array/handler.go Signed-off-by: Future-Outlier --- .../controller/nodes/array/handler_test.go | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/flytepropeller/pkg/controller/nodes/array/handler_test.go b/flytepropeller/pkg/controller/nodes/array/handler_test.go index 648d70e36c..7d44ed60dd 100644 --- a/flytepropeller/pkg/controller/nodes/array/handler_test.go +++ b/flytepropeller/pkg/controller/nodes/array/handler_test.go @@ -3,6 +3,7 @@ package array import ( "context" "fmt" + "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/errors" "testing" "github.com/stretchr/testify/assert" @@ -941,6 +942,68 @@ func TestHandleArrayNodePhaseExecuting(t *testing.T) { } } +func TestHandle_InvalidLiteralType(t *testing.T) { + ctx := context.Background() + scope := promutils.NewTestScope() + dataStore, err := storage.NewDataStore(&storage.Config{ + Type: storage.TypeMemory, + }, scope) + assert.NoError(t, err) + nodeHandler := &mocks.NodeHandler{} + + // Initialize ArrayNodeHandler + arrayNodeHandler, err := createArrayNodeHandler(ctx, t, nodeHandler, dataStore, scope) + assert.NoError(t, err) + + // Test cases + tests := []struct { + name string + inputLiteral *idlcore.Literal + expectedTransitionType handler.TransitionType + expectedPhase handler.EPhase + expectedErrorCode string + expectedErrorMessage string + }{ + { + name: "InvalidLiteralType", + inputLiteral: &idlcore.Literal{ + Value: &idlcore.Literal_Scalar{ + Scalar: &idlcore.Scalar{}, + }, + }, + expectedTransitionType: handler.TransitionTypeEphemeral, + expectedPhase: handler.EPhaseFailed, + expectedErrorCode: errors.IDLNotFoundErr, + expectedErrorMessage: "Input is an invalid type, please update all of your Flyte images to the latest version and try again.", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Create NodeExecutionContext + literalMap := &idlcore.LiteralMap{ + Literals: map[string]*idlcore.Literal{ + "invalidInput": test.inputLiteral, + }, + } + arrayNodeState := &handler.ArrayNodeState{ + Phase: v1alpha1.ArrayNodePhaseNone, + } + nCtx := createNodeExecutionContext(dataStore, newBufferedEventRecorder(), nil, literalMap, &arrayNodeSpec, arrayNodeState, 0, workflowMaxParallelism) + + // Evaluate node + transition, err := arrayNodeHandler.Handle(ctx, nCtx) + assert.NoError(t, err) + + // Validate results + assert.Equal(t, test.expectedTransitionType, transition.Type()) + assert.Equal(t, test.expectedPhase, transition.Info().GetPhase()) + assert.Equal(t, test.expectedErrorCode, transition.Info().GetErr().Code) + assert.Equal(t, test.expectedErrorMessage, transition.Info().GetErr().Message) + }) + } +} + func TestHandleArrayNodePhaseExecutingSubNodeFailures(t *testing.T) { ctx := context.Background() From f5b5d9506ef76ad5f0b669a703f4dafe642fcd1f Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Fri, 30 Aug 2024 16:47:28 +0800 Subject: [PATCH 09/17] fix imports Signed-off-by: Future-Outlier --- flytepropeller/pkg/controller/nodes/array/handler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flytepropeller/pkg/controller/nodes/array/handler_test.go b/flytepropeller/pkg/controller/nodes/array/handler_test.go index 7d44ed60dd..5bcbced6dc 100644 --- a/flytepropeller/pkg/controller/nodes/array/handler_test.go +++ b/flytepropeller/pkg/controller/nodes/array/handler_test.go @@ -3,7 +3,6 @@ package array import ( "context" "fmt" - "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/errors" "testing" "github.com/stretchr/testify/assert" @@ -21,6 +20,7 @@ import ( execmocks "github.com/flyteorg/flyte/flytepropeller/pkg/controller/executors/mocks" "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes" "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/catalog" + "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/errors" gatemocks "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/gate/mocks" "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/handler" "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/interfaces" From fe58c078b40c2b5665a46dbbe0916f5784a6ca9c Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Fri, 30 Aug 2024 17:14:17 +0800 Subject: [PATCH 10/17] add tests for k8s/inputs and catalog/datacatalog/transformer Signed-off-by: Future-Outlier --- .../pkg/compiler/errors/compiler_errors.go | 2 +- .../compiler/transformers/k8s/inputs_test.go | 54 +++++++++++++++++++ .../nodes/catalog/datacatalog/transformer.go | 2 +- .../catalog/datacatalog/transformer_test.go | 40 ++++++++++++++ 4 files changed, 96 insertions(+), 2 deletions(-) diff --git a/flytepropeller/pkg/compiler/errors/compiler_errors.go b/flytepropeller/pkg/compiler/errors/compiler_errors.go index 16b55249a7..656e9db21f 100755 --- a/flytepropeller/pkg/compiler/errors/compiler_errors.go +++ b/flytepropeller/pkg/compiler/errors/compiler_errors.go @@ -224,7 +224,7 @@ func NewMismatchingVariablesErr(nodeID, fromVar, fromType, toVar, toType string) func NewIDLTypeNotFoundErr(nodeID string) *CompileError { return newError( IDLTypeNotFoundError, - "Input is an invalid type, please update all of your Flyte images to the latest version and try again.", + "Input type IDL is not found, please update all of your Flyte images to the latest version and try again.", nodeID, ) } diff --git a/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go b/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go index 01d667c35d..f0d33ab281 100644 --- a/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go +++ b/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go @@ -1 +1,55 @@ package k8s + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core" + "github.com/flyteorg/flyte/flytepropeller/pkg/compiler/common" + "github.com/flyteorg/flyte/flytepropeller/pkg/compiler/errors" +) + +func TestValidateInputs_IDLTypeNotFound(t *testing.T) { + nodeID := common.NodeID("test-node") + + iface := &core.TypedInterface{ + Inputs: &core.VariableMap{ + Variables: map[string]*core.Variable{ + "input1": { + Type: &core.LiteralType{ + Type: &core.LiteralType_Simple{ + Simple: 1000, + }, + }, + }, + }, + }, + } + + inputs := core.LiteralMap{ + Literals: map[string]*core.Literal{ + "input1": &core.Literal{}, // This will cause LiteralTypeForLiteral to return nil + }, + } + + errs := errors.NewCompileErrors() + ok := validateInputs(nodeID, iface, inputs, errs) + + assert.False(t, ok) + assert.True(t, errs.HasErrors()) + + idlNotFound := false + var errMsg string + for _, err := range errs.Errors().List() { + if err.Code() == "IDLTypeNotFound" { + idlNotFound = true + errMsg = err.Error() + break + } + } + assert.True(t, idlNotFound, "Expected IDLTypeNotFound error was not found in errors") + + expectedErrMsg := "Code: IDLTypeNotFound, Node Id: test-node, Description: Input type IDL is not found, please update all of your Flyte images to the latest version and try again." + assert.Equal(t, expectedErrMsg, errMsg, "Error message does not match expected value") +} diff --git a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go index 980d922f78..31b6e0aeda 100644 --- a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go +++ b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go @@ -55,7 +55,7 @@ func GenerateTaskOutputsFromArtifact(id core.Identifier, taskInterface core.Type expectedVarType := outputVariables[artifactData.Name].GetType() inputType := validators.LiteralTypeForLiteral(artifactData.Value) if inputType == nil { - return nil, fmt.Errorf("please update all of your Flyte images to the latest version and try again") + return nil, fmt.Errorf("IDL not found, please update all of your Flyte images to the latest version and try again") } if !validators.AreTypesCastable(inputType, expectedVarType) { return nil, fmt.Errorf("unexpected artifactData: [%v] type: [%v] does not match any task output type: [%v]", artifactData.Name, inputType, expectedVarType) diff --git a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer_test.go b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer_test.go index 1c6b9e2e1b..b0d3e9c1dc 100644 --- a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer_test.go +++ b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer_test.go @@ -331,3 +331,43 @@ func TestDatasetIDToIdentifier(t *testing.T) { assert.Equal(t, "d", id.Domain) assert.Equal(t, "v", id.Version) } + +func TestGenerateTaskOutputsFromArtifact_IDLNotFound(t *testing.T) { + taskID := core.Identifier{ + ResourceType: core.ResourceType_TASK, + Project: "project", + Domain: "domain", + Name: "name", + Version: "version", + } + + taskInterface := core.TypedInterface{ + Outputs: &core.VariableMap{ + Variables: map[string]*core.Variable{ + "output1": { + Type: &core.LiteralType{ + Type: &core.LiteralType_Simple{ + Simple: 1000, + }, + }, + }, + }, + }, + } + + artifact := &datacatalog.Artifact{ + Id: "artifact_id", + Data: []*datacatalog.ArtifactData{ + { + Name: "output1", + Value: &core.Literal{}, // This will cause LiteralTypeForLiteral to return nil + }, + }, + } + + _, err := GenerateTaskOutputsFromArtifact(taskID, taskInterface, artifact) + + expectedError := "IDL not found, please update all of your Flyte images to the latest version and try again" + assert.Error(t, err) + assert.Equal(t, expectedError, err.Error()) +} From 8ae97c2f80e24066da55525aa70ab26f7f53f2ef Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Thu, 5 Sep 2024 00:41:49 -0700 Subject: [PATCH 11/17] kevin's change Signed-off-by: Kevin Su --- .../pkg/manager/impl/validation/validation.go | 16 +++------------- flytepropeller/pkg/compiler/validators/utils.go | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/flyteadmin/pkg/manager/impl/validation/validation.go b/flyteadmin/pkg/manager/impl/validation/validation.go index 55c45db9bb..c5f22fba14 100644 --- a/flyteadmin/pkg/manager/impl/validation/validation.go +++ b/flyteadmin/pkg/manager/impl/validation/validation.go @@ -283,19 +283,9 @@ func validateParameterMap(inputMap *core.ParameterMap, fieldName string) error { defaultValue := defaultInput.GetDefault() if defaultValue != nil { inputType := validators.LiteralTypeForLiteral(defaultValue) - - if inputType == nil { - return errors.NewFlyteAdminErrorf(codes.InvalidArgument, - fmt.Sprintf( - "Flyte encountered an issue while determining\n"+ - "the type of the default value for Parameter '%s' in '%s'.\n"+ - "Registered type: [%s].\n"+ - "Flyte needs to support the latest FlyteIDL to support this type.\n"+ - "Suggested solution: Please update all of your Flyte images to the latest version and "+ - "try again.", - name, fieldName, defaultInput.GetVar().GetType().String(), - ), - ) + err := validators.ValidateLiteralType(inputType) + if err != nil { + return errors.NewFlyteAdminErrorf(codes.InvalidArgument, fmt.Sprintf("Failed to validate literal type for %s in %s with err: %s", name, fieldName, err)) } if !validators.AreTypesCastable(inputType, defaultInput.GetVar().GetType()) { diff --git a/flytepropeller/pkg/compiler/validators/utils.go b/flytepropeller/pkg/compiler/validators/utils.go index 5f41a6e65e..5815cb4f0c 100644 --- a/flytepropeller/pkg/compiler/validators/utils.go +++ b/flytepropeller/pkg/compiler/validators/utils.go @@ -242,6 +242,23 @@ func literalTypeForLiterals(literals []*core.Literal) *core.LiteralType { return buildMultipleTypeUnion(innerType) } +// ValidateLiteralType check if the literal type is valid, return error if the literal is invalid. +func ValidateLiteralType(lt *core.LiteralType) error { + err := fmt.Errorf("got unknown literal: [%v].\n"+ + "Suggested solution: Please update all your Flyte deployment images to the latest version and try again.", l) + if lt == nil { + return err + } + if lt.GetCollectionType() != nil { + return ValidateLiteralType(lt.GetCollectionType()) + } + if lt.GetMapValueType() != nil { + return ValidateLiteralType(lt.GetMapValueType()) + } + + return nil +} + // LiteralTypeForLiteral gets LiteralType for literal, nil if the value of literal is unknown, or type collection/map of // type None if the literal is a non-homogeneous type. func LiteralTypeForLiteral(l *core.Literal) *core.LiteralType { From 20c37872cc1c052ca5b5f70fab702eb68d7640e3 Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Tue, 24 Sep 2024 17:51:45 +0800 Subject: [PATCH 12/17] update all pingsu's interface Signed-off-by: Future-Outlier --- .../impl/validation/execution_validator.go | 12 +++------- .../validation/execution_validator_test.go | 6 +++-- .../impl/validation/launch_plan_validator.go | 12 +++------- .../validation/launch_plan_validator_test.go | 4 ++-- .../impl/validation/signal_validator.go | 6 +++++ .../impl/validation/signal_validator_test.go | 8 ++----- .../pkg/manager/impl/validation/validation.go | 3 ++- .../impl/validation/validation_test.go | 11 +--------- .../compiler/errors/compiler_error_test.go | 2 +- .../pkg/compiler/errors/compiler_errors.go | 4 ++-- .../pkg/compiler/transformers/k8s/inputs.go | 10 +++++++-- .../compiler/transformers/k8s/inputs_test.go | 6 ++--- .../pkg/compiler/validators/utils.go | 4 ++-- .../pkg/controller/nodes/array/handler.go | 8 ++++--- .../controller/nodes/array/handler_test.go | 22 +++++++++---------- .../nodes/catalog/datacatalog/transformer.go | 5 +++-- .../catalog/datacatalog/transformer_test.go | 4 ++-- 17 files changed, 60 insertions(+), 67 deletions(-) diff --git a/flyteadmin/pkg/manager/impl/validation/execution_validator.go b/flyteadmin/pkg/manager/impl/validation/execution_validator.go index f128ee7c11..b3424fec21 100644 --- a/flyteadmin/pkg/manager/impl/validation/execution_validator.go +++ b/flyteadmin/pkg/manager/impl/validation/execution_validator.go @@ -108,16 +108,10 @@ func CheckAndFetchInputsForExecution( default: inputType = validators.LiteralTypeForLiteral(executionInputMap[name]) } - if inputType == nil { + err := validators.ValidateLiteralType(inputType) + if err != nil { return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, - fmt.Sprintf( - "invalid %s input wrong type.\n"+ - "Expected %s, but got %s.\n"+ - "Suggested solution: Please update all of your Flyte images to the latest version and try"+ - " again.", - name, expectedInput.GetVar().GetType().String(), inputType, - ), - ) + fmt.Sprintf("Failed to validate literal type for %s with err: %s", name, err)) } if !validators.AreTypesCastable(inputType, expectedInput.GetVar().GetType()) { return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "invalid %s input wrong type. Expected %s, but got %s", name, expectedInput.GetVar().GetType(), inputType) diff --git a/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go b/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go index 5935bba5d6..943e5006e7 100644 --- a/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go +++ b/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go @@ -17,6 +17,8 @@ import ( var execConfig = testutils.GetApplicationConfigWithDefaultDomains() +const failedToValidateLiteralType = "Failed to validate literal type" + func TestValidateExecEmptyProject(t *testing.T) { request := testutils.GetExecutionRequest() request.Project = "" @@ -241,8 +243,8 @@ func TestValidateExecUnknownIDLInputs(t *testing.T) { ) assert.NotNil(t, err) - expectedErrorMsg := "invalid foo input wrong type.\nExpected simple:1000, but got .\nSuggested solution: Please update all of your Flyte images to the latest version and try again." - assert.Equal(t, expectedErrorMsg, err.Error()) + // Expected error message + assert.Contains(t, err.Error(), failedToValidateLiteralType) } func TestValidExecutionId(t *testing.T) { diff --git a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go index 650537dabb..d2dc061e64 100644 --- a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go +++ b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go @@ -144,16 +144,10 @@ func checkAndFetchExpectedInputForLaunchPlan( return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "unexpected fixed_input %s", name) } inputType := validators.LiteralTypeForLiteral(fixedInput) - if inputType == nil { + err := validators.ValidateLiteralType(inputType) + if err != nil { return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, - fmt.Sprintf( - "invalid %s input wrong type.\n"+ - "Expected %s, but got %s.\n"+ - "Suggested solution: Please update all of your Flyte images to the latest version and try"+ - " again.", - name, value.GetType().String(), inputType, - ), - ) + fmt.Sprintf("Failed to validate literal type for %s with err: %s", name, err)) } if !validators.AreTypesCastable(inputType, value.GetType()) { return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, diff --git a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go index 7a4a7735d6..ab2832eeeb 100644 --- a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go +++ b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go @@ -271,8 +271,8 @@ func TestGetLpExpectedInvalidFixedInputWithUnknownIDL(t *testing.T) { assert.NotNil(t, err) - expectedErrorMsg := "invalid foo input wrong type.\nExpected simple:1000, but got .\nSuggested solution: Please update all of your Flyte images to the latest version and try again." - assert.Equal(t, expectedErrorMsg, err.Error()) + // Expected error message + assert.Contains(t, err.Error(), failedToValidateLiteralType) } func TestGetLpExpectedNoFixedInput(t *testing.T) { diff --git a/flyteadmin/pkg/manager/impl/validation/signal_validator.go b/flyteadmin/pkg/manager/impl/validation/signal_validator.go index 5139b6f866..4daa62acb9 100644 --- a/flyteadmin/pkg/manager/impl/validation/signal_validator.go +++ b/flyteadmin/pkg/manager/impl/validation/signal_validator.go @@ -77,6 +77,12 @@ func ValidateSignalSetRequest(ctx context.Context, db repositoryInterfaces.Repos if err != nil { return err } + err = propellervalidators.ValidateLiteralType(valueType) + if err != nil { + return errors.NewFlyteAdminErrorf(codes.InvalidArgument, + fmt.Sprintf("Failed to validate literal type for %s with err: %s", valueType, err)) + } + if valueType == nil { return errors.NewFlyteAdminErrorf(codes.InvalidArgument, fmt.Sprintf( diff --git a/flyteadmin/pkg/manager/impl/validation/signal_validator_test.go b/flyteadmin/pkg/manager/impl/validation/signal_validator_test.go index 635a610d81..c78c2c366b 100644 --- a/flyteadmin/pkg/manager/impl/validation/signal_validator_test.go +++ b/flyteadmin/pkg/manager/impl/validation/signal_validator_test.go @@ -325,14 +325,10 @@ func TestValidateSignalUpdateRequest(t *testing.T) { } // Invoke the function and check for the expected error - err := ValidateSignalSetRequest(ctx, repo, request) + err := ValidateSignalSetRequest(ctx, repo, &request) assert.NotNil(t, err) // Expected error message - expectedErrorMsg := "Invalid signal value for 'scalar:{}'.\n" + - "Expected type: simple:1000, but received: .\n" + - "Suggested solution: Ensure all Flyte images are updated to the latest version and try again." - assert.Equal(t, expectedErrorMsg, err.Error()) - + assert.Contains(t, err.Error(), failedToValidateLiteralType) }) } diff --git a/flyteadmin/pkg/manager/impl/validation/validation.go b/flyteadmin/pkg/manager/impl/validation/validation.go index d75323cba9..a26f11eed7 100644 --- a/flyteadmin/pkg/manager/impl/validation/validation.go +++ b/flyteadmin/pkg/manager/impl/validation/validation.go @@ -285,7 +285,8 @@ func validateParameterMap(inputMap *core.ParameterMap, fieldName string) error { inputType := validators.LiteralTypeForLiteral(defaultValue) err := validators.ValidateLiteralType(inputType) if err != nil { - return errors.NewFlyteAdminErrorf(codes.InvalidArgument, fmt.Sprintf("Failed to validate literal type for %s in %s with err: %s", name, fieldName, err)) + return errors.NewFlyteAdminErrorf(codes.InvalidArgument, + fmt.Sprintf("Failed to validate literal type for %s with err: %s", name, err)) } if !validators.AreTypesCastable(inputType, defaultInput.GetVar().GetType()) { diff --git a/flyteadmin/pkg/manager/impl/validation/validation_test.go b/flyteadmin/pkg/manager/impl/validation/validation_test.go index 0cc20cfd1b..265868789e 100644 --- a/flyteadmin/pkg/manager/impl/validation/validation_test.go +++ b/flyteadmin/pkg/manager/impl/validation/validation_test.go @@ -347,16 +347,7 @@ func TestValidateParameterMap(t *testing.T) { err := validateParameterMap(&exampleMap, fieldName) assert.Error(t, err) fmt.Println(err.Error()) - expectedErrMsg := fmt.Sprintf( - "Flyte encountered an issue while determining\n"+ - "the type of the default value for Parameter '%s' in '%s'.\n"+ - "Registered type: [%s].\n"+ - "Flyte needs to support the latest FlyteIDL to support this type.\n"+ - "Suggested solution: Please update all of your Flyte images to the latest version and "+ - "try again.", - name, fieldName, exampleMap.Parameters[name].GetVar().GetType().String(), - ) - assert.Equal(t, expectedErrMsg, err.Error()) + assert.Contains(t, err.Error(), failedToValidateLiteralType) }) } diff --git a/flytepropeller/pkg/compiler/errors/compiler_error_test.go b/flytepropeller/pkg/compiler/errors/compiler_error_test.go index aee616ad88..e382f41f26 100644 --- a/flytepropeller/pkg/compiler/errors/compiler_error_test.go +++ b/flytepropeller/pkg/compiler/errors/compiler_error_test.go @@ -33,7 +33,7 @@ func TestErrorCodes(t *testing.T) { UnrecognizedValue: NewUnrecognizedValueErr("", ""), WorkflowBuildError: NewWorkflowBuildError(errors.New("")), NoNodesFound: NewNoNodesFoundErr(""), - IDLTypeNotFoundError: NewIDLTypeNotFoundErr(""), + IDLTypeNotFoundError: NewIDLTypeNotFoundErr("", ""), } for key, value := range testCases { diff --git a/flytepropeller/pkg/compiler/errors/compiler_errors.go b/flytepropeller/pkg/compiler/errors/compiler_errors.go index 656e9db21f..605542b333 100755 --- a/flytepropeller/pkg/compiler/errors/compiler_errors.go +++ b/flytepropeller/pkg/compiler/errors/compiler_errors.go @@ -221,10 +221,10 @@ func NewMismatchingVariablesErr(nodeID, fromVar, fromType, toVar, toType string) ) } -func NewIDLTypeNotFoundErr(nodeID string) *CompileError { +func NewIDLTypeNotFoundErr(nodeID, errMsg string) *CompileError { return newError( IDLTypeNotFoundError, - "Input type IDL is not found, please update all of your Flyte images to the latest version and try again.", + errMsg, nodeID, ) } diff --git a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go index 65db02e6d6..02db86c7f8 100644 --- a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go +++ b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go @@ -1,6 +1,8 @@ package k8s import ( + "fmt" + "k8s.io/apimachinery/pkg/util/sets" "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core" @@ -42,10 +44,14 @@ func validateInputs(nodeID common.NodeID, iface *core.TypedInterface, inputs cor default: inputType = validators.LiteralTypeForLiteral(inputVal) } - if inputType == nil { - errs.Collect(errors.NewIDLTypeNotFoundErr(nodeID)) + + err := validators.ValidateLiteralType(inputType) + if err != nil { + errMsg := fmt.Sprintf("Failed to validate literal type for %s with err: %s", inputVar, err) + errs.Collect(errors.NewIDLTypeNotFoundErr(nodeID, errMsg)) continue } + if !validators.AreTypesCastable(inputType, v.Type) { errs.Collect(errors.NewMismatchingTypesErr(nodeID, inputVar, v.Type.String(), inputType.String())) continue diff --git a/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go b/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go index f0d33ab281..22eb7f2ebf 100644 --- a/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go +++ b/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go @@ -29,7 +29,7 @@ func TestValidateInputs_IDLTypeNotFound(t *testing.T) { inputs := core.LiteralMap{ Literals: map[string]*core.Literal{ - "input1": &core.Literal{}, // This will cause LiteralTypeForLiteral to return nil + "input1": nil, // Set this to nil to trigger the nil case }, } @@ -50,6 +50,6 @@ func TestValidateInputs_IDLTypeNotFound(t *testing.T) { } assert.True(t, idlNotFound, "Expected IDLTypeNotFound error was not found in errors") - expectedErrMsg := "Code: IDLTypeNotFound, Node Id: test-node, Description: Input type IDL is not found, please update all of your Flyte images to the latest version and try again." - assert.Equal(t, expectedErrMsg, errMsg, "Error message does not match expected value") + expectedContainedErrorMsg := "Failed to validate literal type" + assert.Contains(t, errMsg, expectedContainedErrorMsg) } diff --git a/flytepropeller/pkg/compiler/validators/utils.go b/flytepropeller/pkg/compiler/validators/utils.go index 7e47fc7fae..fb05e32bb3 100644 --- a/flytepropeller/pkg/compiler/validators/utils.go +++ b/flytepropeller/pkg/compiler/validators/utils.go @@ -251,9 +251,9 @@ func literalTypeForLiterals(literals []*core.Literal) *core.LiteralType { // ValidateLiteralType check if the literal type is valid, return error if the literal is invalid. func ValidateLiteralType(lt *core.LiteralType) error { - err := fmt.Errorf("got unknown literal: [%v].\n"+ - "Suggested solution: Please update all your Flyte deployment images to the latest version and try again.", l) if lt == nil { + err := fmt.Errorf("got unknown literal type: [%v].\n"+ + "Suggested solution: Please update all your Flyte deployment images to the latest version and try again", lt) return err } if lt.GetCollectionType() != nil { diff --git a/flytepropeller/pkg/controller/nodes/array/handler.go b/flytepropeller/pkg/controller/nodes/array/handler.go index fccf5fe62c..29e4e6c10f 100644 --- a/flytepropeller/pkg/controller/nodes/array/handler.go +++ b/flytepropeller/pkg/controller/nodes/array/handler.go @@ -191,11 +191,13 @@ func (a *arrayNodeHandler) Handle(ctx context.Context, nCtx interfaces.NodeExecu } size := -1 - for _, variable := range literalMap.Literals { + for key, variable := range literalMap.Literals { literalType := validators.LiteralTypeForLiteral(variable) - if literalType == nil { + err := validators.ValidateLiteralType(literalType) + if err != nil { + errMsg := fmt.Sprintf("Failed to validate literal type for %s with err: %s", key, err) return handler.DoTransition(handler.TransitionTypeEphemeral, - handler.PhaseInfoFailure(idlcore.ExecutionError_USER, errors.IDLNotFoundErr, "Input is an invalid type, please update all of your Flyte images to the latest version and try again.", nil), + handler.PhaseInfoFailure(idlcore.ExecutionError_USER, errors.IDLNotFoundErr, errMsg, nil), ), nil } switch literalType.Type.(type) { diff --git a/flytepropeller/pkg/controller/nodes/array/handler_test.go b/flytepropeller/pkg/controller/nodes/array/handler_test.go index dbad960f48..08eea22e09 100644 --- a/flytepropeller/pkg/controller/nodes/array/handler_test.go +++ b/flytepropeller/pkg/controller/nodes/array/handler_test.go @@ -959,12 +959,12 @@ func TestHandle_InvalidLiteralType(t *testing.T) { // Test cases tests := []struct { - name string - inputLiteral *idlcore.Literal - expectedTransitionType handler.TransitionType - expectedPhase handler.EPhase - expectedErrorCode string - expectedErrorMessage string + name string + inputLiteral *idlcore.Literal + expectedTransitionType handler.TransitionType + expectedPhase handler.EPhase + expectedErrorCode string + expectedContainedErrorMsg string }{ { name: "InvalidLiteralType", @@ -973,10 +973,10 @@ func TestHandle_InvalidLiteralType(t *testing.T) { Scalar: &idlcore.Scalar{}, }, }, - expectedTransitionType: handler.TransitionTypeEphemeral, - expectedPhase: handler.EPhaseFailed, - expectedErrorCode: errors.IDLNotFoundErr, - expectedErrorMessage: "Input is an invalid type, please update all of your Flyte images to the latest version and try again.", + expectedTransitionType: handler.TransitionTypeEphemeral, + expectedPhase: handler.EPhaseFailed, + expectedErrorCode: errors.IDLNotFoundErr, + expectedContainedErrorMsg: "Failed to validate literal type", }, } @@ -1001,7 +1001,7 @@ func TestHandle_InvalidLiteralType(t *testing.T) { assert.Equal(t, test.expectedTransitionType, transition.Type()) assert.Equal(t, test.expectedPhase, transition.Info().GetPhase()) assert.Equal(t, test.expectedErrorCode, transition.Info().GetErr().Code) - assert.Equal(t, test.expectedErrorMessage, transition.Info().GetErr().Message) + assert.Contains(t, transition.Info().GetErr().Message, test.expectedContainedErrorMsg) }) } } diff --git a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go index 31b6e0aeda..ba94bdadec 100644 --- a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go +++ b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go @@ -54,8 +54,9 @@ func GenerateTaskOutputsFromArtifact(id core.Identifier, taskInterface core.Type expectedVarType := outputVariables[artifactData.Name].GetType() inputType := validators.LiteralTypeForLiteral(artifactData.Value) - if inputType == nil { - return nil, fmt.Errorf("IDL not found, please update all of your Flyte images to the latest version and try again") + err := validators.ValidateLiteralType(inputType) + if err != nil { + return nil, fmt.Errorf("failed to validate literal type for %s with err: %s", artifactData.Name, err) } if !validators.AreTypesCastable(inputType, expectedVarType) { return nil, fmt.Errorf("unexpected artifactData: [%v] type: [%v] does not match any task output type: [%v]", artifactData.Name, inputType, expectedVarType) diff --git a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer_test.go b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer_test.go index b0d3e9c1dc..92e4c82926 100644 --- a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer_test.go +++ b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer_test.go @@ -367,7 +367,7 @@ func TestGenerateTaskOutputsFromArtifact_IDLNotFound(t *testing.T) { _, err := GenerateTaskOutputsFromArtifact(taskID, taskInterface, artifact) - expectedError := "IDL not found, please update all of your Flyte images to the latest version and try again" + expectedContainedErrorMsg := "failed to validate literal type" assert.Error(t, err) - assert.Equal(t, expectedError, err.Error()) + assert.Contains(t, err.Error(), expectedContainedErrorMsg) } From c4914be0a9c36609d6f1888d42567d99c35f4536 Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Tue, 24 Sep 2024 18:05:30 +0800 Subject: [PATCH 13/17] Trigger Build Signed-off-by: Future-Outlier From dfb09b04781cc9506d853ed0efb967776ad4876a Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Wed, 25 Sep 2024 10:27:01 +0800 Subject: [PATCH 14/17] update pingsu's advice Signed-off-by: Future-Outlier Co-authored-by: pingsutw --- flyteadmin/pkg/errors/errors.go | 5 +++++ .../impl/validation/execution_validator.go | 4 +--- .../impl/validation/launch_plan_validator.go | 4 +--- .../manager/impl/validation/signal_validator.go | 15 +-------------- .../pkg/manager/impl/validation/validation.go | 4 +--- 5 files changed, 9 insertions(+), 23 deletions(-) diff --git a/flyteadmin/pkg/errors/errors.go b/flyteadmin/pkg/errors/errors.go index 0fd9542ba7..5fc48b0b67 100644 --- a/flyteadmin/pkg/errors/errors.go +++ b/flyteadmin/pkg/errors/errors.go @@ -213,3 +213,8 @@ func NewInactiveProjectError(ctx context.Context, id string) FlyteAdminError { } return statusErr } + +func NewInvalidLiteralTypeError(name string, err error) FlyteAdminError { + return NewFlyteAdminErrorf(codes.InvalidArgument, + fmt.Sprintf("Failed to validate literal type for [%s] with err: %s", name, err)) +} diff --git a/flyteadmin/pkg/manager/impl/validation/execution_validator.go b/flyteadmin/pkg/manager/impl/validation/execution_validator.go index b3424fec21..639f29073f 100644 --- a/flyteadmin/pkg/manager/impl/validation/execution_validator.go +++ b/flyteadmin/pkg/manager/impl/validation/execution_validator.go @@ -2,7 +2,6 @@ package validation import ( "context" - "fmt" "regexp" "strings" @@ -110,8 +109,7 @@ func CheckAndFetchInputsForExecution( } err := validators.ValidateLiteralType(inputType) if err != nil { - return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, - fmt.Sprintf("Failed to validate literal type for %s with err: %s", name, err)) + return nil, errors.NewInvalidLiteralTypeError(name, err) } if !validators.AreTypesCastable(inputType, expectedInput.GetVar().GetType()) { return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "invalid %s input wrong type. Expected %s, but got %s", name, expectedInput.GetVar().GetType(), inputType) diff --git a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go index d2dc061e64..2a49b4da87 100644 --- a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go +++ b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go @@ -2,7 +2,6 @@ package validation import ( "context" - "fmt" "google.golang.org/grpc/codes" @@ -146,8 +145,7 @@ func checkAndFetchExpectedInputForLaunchPlan( inputType := validators.LiteralTypeForLiteral(fixedInput) err := validators.ValidateLiteralType(inputType) if err != nil { - return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, - fmt.Sprintf("Failed to validate literal type for %s with err: %s", name, err)) + return nil, errors.NewInvalidLiteralTypeError(name, err) } if !validators.AreTypesCastable(inputType, value.GetType()) { return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, diff --git a/flyteadmin/pkg/manager/impl/validation/signal_validator.go b/flyteadmin/pkg/manager/impl/validation/signal_validator.go index 4daa62acb9..af1d4425aa 100644 --- a/flyteadmin/pkg/manager/impl/validation/signal_validator.go +++ b/flyteadmin/pkg/manager/impl/validation/signal_validator.go @@ -2,7 +2,6 @@ package validation import ( "context" - "fmt" "google.golang.org/grpc/codes" @@ -79,19 +78,7 @@ func ValidateSignalSetRequest(ctx context.Context, db repositoryInterfaces.Repos } err = propellervalidators.ValidateLiteralType(valueType) if err != nil { - return errors.NewFlyteAdminErrorf(codes.InvalidArgument, - fmt.Sprintf("Failed to validate literal type for %s with err: %s", valueType, err)) - } - - if valueType == nil { - return errors.NewFlyteAdminErrorf(codes.InvalidArgument, - fmt.Sprintf( - "Invalid signal value for '%s'.\n"+ - "Expected type: %s, but received: %s.\n"+ - "Suggested solution: Ensure all Flyte images are updated to the latest version and try again.", - request.Value, lookupSignal.GetType().String(), valueType, - ), - ) + return errors.NewInvalidLiteralTypeError("", err) } if !propellervalidators.AreTypesCastable(lookupSignal.Type, valueType) { return errors.NewFlyteAdminErrorf(codes.InvalidArgument, diff --git a/flyteadmin/pkg/manager/impl/validation/validation.go b/flyteadmin/pkg/manager/impl/validation/validation.go index a26f11eed7..de2927495c 100644 --- a/flyteadmin/pkg/manager/impl/validation/validation.go +++ b/flyteadmin/pkg/manager/impl/validation/validation.go @@ -1,7 +1,6 @@ package validation import ( - "fmt" "net/url" "strconv" "strings" @@ -285,8 +284,7 @@ func validateParameterMap(inputMap *core.ParameterMap, fieldName string) error { inputType := validators.LiteralTypeForLiteral(defaultValue) err := validators.ValidateLiteralType(inputType) if err != nil { - return errors.NewFlyteAdminErrorf(codes.InvalidArgument, - fmt.Sprintf("Failed to validate literal type for %s with err: %s", name, err)) + return errors.NewInvalidLiteralTypeError(name, err) } if !validators.AreTypesCastable(inputType, defaultInput.GetVar().GetType()) { From 5d2e06240ae88a4b11fed2b91f5ce56f7a6d9231 Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Wed, 25 Sep 2024 10:33:08 +0800 Subject: [PATCH 15/17] nit Signed-off-by: Future-Outlier --- flytepropeller/pkg/compiler/transformers/k8s/inputs.go | 2 +- flytepropeller/pkg/controller/nodes/array/handler.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go index 02db86c7f8..2fc1a0ec4e 100644 --- a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go +++ b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go @@ -47,7 +47,7 @@ func validateInputs(nodeID common.NodeID, iface *core.TypedInterface, inputs cor err := validators.ValidateLiteralType(inputType) if err != nil { - errMsg := fmt.Sprintf("Failed to validate literal type for %s with err: %s", inputVar, err) + errMsg := fmt.Sprintf("Failed to validate literal type for [%s] with err: %s", inputVar, err) errs.Collect(errors.NewIDLTypeNotFoundErr(nodeID, errMsg)) continue } diff --git a/flytepropeller/pkg/controller/nodes/array/handler.go b/flytepropeller/pkg/controller/nodes/array/handler.go index 29e4e6c10f..6859bf46f0 100644 --- a/flytepropeller/pkg/controller/nodes/array/handler.go +++ b/flytepropeller/pkg/controller/nodes/array/handler.go @@ -195,7 +195,7 @@ func (a *arrayNodeHandler) Handle(ctx context.Context, nCtx interfaces.NodeExecu literalType := validators.LiteralTypeForLiteral(variable) err := validators.ValidateLiteralType(literalType) if err != nil { - errMsg := fmt.Sprintf("Failed to validate literal type for %s with err: %s", key, err) + errMsg := fmt.Sprintf("Failed to validate literal type for [%s] with err: %s", key, err) return handler.DoTransition(handler.TransitionTypeEphemeral, handler.PhaseInfoFailure(idlcore.ExecutionError_USER, errors.IDLNotFoundErr, errMsg, nil), ), nil From c8429fa5fee11d7e4424081802d44346de44d9b8 Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Wed, 25 Sep 2024 14:02:17 +0800 Subject: [PATCH 16/17] use InvalidLiteralTypeErr instead of NewIDLTypeNotFoundErr Signed-off-by: Future-Outlier --- flytepropeller/pkg/compiler/errors/compiler_error_test.go | 2 +- flytepropeller/pkg/compiler/errors/compiler_errors.go | 6 +++--- flytepropeller/pkg/compiler/transformers/k8s/inputs.go | 2 +- flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/flytepropeller/pkg/compiler/errors/compiler_error_test.go b/flytepropeller/pkg/compiler/errors/compiler_error_test.go index e382f41f26..a86b14df06 100644 --- a/flytepropeller/pkg/compiler/errors/compiler_error_test.go +++ b/flytepropeller/pkg/compiler/errors/compiler_error_test.go @@ -33,7 +33,7 @@ func TestErrorCodes(t *testing.T) { UnrecognizedValue: NewUnrecognizedValueErr("", ""), WorkflowBuildError: NewWorkflowBuildError(errors.New("")), NoNodesFound: NewNoNodesFoundErr(""), - IDLTypeNotFoundError: NewIDLTypeNotFoundErr("", ""), + InvalidLiteralTypeError: NewInvalidLiteralTypeErr("", ""), } for key, value := range testCases { diff --git a/flytepropeller/pkg/compiler/errors/compiler_errors.go b/flytepropeller/pkg/compiler/errors/compiler_errors.go index 605542b333..3f3d780e89 100755 --- a/flytepropeller/pkg/compiler/errors/compiler_errors.go +++ b/flytepropeller/pkg/compiler/errors/compiler_errors.go @@ -98,7 +98,7 @@ const ( FieldNotFoundError ErrorCode = "FieldNotFound" // IDL not found when variable binding - IDLTypeNotFoundError ErrorCode = "IDLTypeNotFound" + InvalidLiteralTypeError ErrorCode = "InvalidLiteralType" ) func NewBranchNodeNotSpecified(branchNodeID string) *CompileError { @@ -221,9 +221,9 @@ func NewMismatchingVariablesErr(nodeID, fromVar, fromType, toVar, toType string) ) } -func NewIDLTypeNotFoundErr(nodeID, errMsg string) *CompileError { +func NewInvalidLiteralTypeErr(nodeID, errMsg string) *CompileError { return newError( - IDLTypeNotFoundError, + InvalidLiteralTypeError, errMsg, nodeID, ) diff --git a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go index 2fc1a0ec4e..7063ad3f2e 100644 --- a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go +++ b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go @@ -48,7 +48,7 @@ func validateInputs(nodeID common.NodeID, iface *core.TypedInterface, inputs cor err := validators.ValidateLiteralType(inputType) if err != nil { errMsg := fmt.Sprintf("Failed to validate literal type for [%s] with err: %s", inputVar, err) - errs.Collect(errors.NewIDLTypeNotFoundErr(nodeID, errMsg)) + errs.Collect(errors.NewInvalidLiteralTypeErr(nodeID, errMsg)) continue } diff --git a/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go b/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go index 22eb7f2ebf..d77aafec49 100644 --- a/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go +++ b/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go @@ -10,7 +10,7 @@ import ( "github.com/flyteorg/flyte/flytepropeller/pkg/compiler/errors" ) -func TestValidateInputs_IDLTypeNotFound(t *testing.T) { +func TestValidateInputs_InvalidLiteralType(t *testing.T) { nodeID := common.NodeID("test-node") iface := &core.TypedInterface{ @@ -42,13 +42,13 @@ func TestValidateInputs_IDLTypeNotFound(t *testing.T) { idlNotFound := false var errMsg string for _, err := range errs.Errors().List() { - if err.Code() == "IDLTypeNotFound" { + if err.Code() == "InvalidLiteralType" { idlNotFound = true errMsg = err.Error() break } } - assert.True(t, idlNotFound, "Expected IDLTypeNotFound error was not found in errors") + assert.True(t, idlNotFound, "Expected InvalidLiteralType error was not found in errors") expectedContainedErrorMsg := "Failed to validate literal type" assert.Contains(t, errMsg, expectedContainedErrorMsg) From 455d50ce60793beca08244abe69adcb77170895e Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Tue, 24 Sep 2024 23:43:49 -0700 Subject: [PATCH 17/17] nit Signed-off-by: Kevin Su --- flytepropeller/pkg/compiler/errors/compiler_error_test.go | 2 +- flytepropeller/pkg/compiler/errors/compiler_errors.go | 4 ++-- flytepropeller/pkg/compiler/transformers/k8s/inputs.go | 5 +---- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/flytepropeller/pkg/compiler/errors/compiler_error_test.go b/flytepropeller/pkg/compiler/errors/compiler_error_test.go index a86b14df06..8394ed5bb7 100644 --- a/flytepropeller/pkg/compiler/errors/compiler_error_test.go +++ b/flytepropeller/pkg/compiler/errors/compiler_error_test.go @@ -33,7 +33,7 @@ func TestErrorCodes(t *testing.T) { UnrecognizedValue: NewUnrecognizedValueErr("", ""), WorkflowBuildError: NewWorkflowBuildError(errors.New("")), NoNodesFound: NewNoNodesFoundErr(""), - InvalidLiteralTypeError: NewInvalidLiteralTypeErr("", ""), + InvalidLiteralTypeError: NewInvalidLiteralTypeErr("", "", errors.New("")), } for key, value := range testCases { diff --git a/flytepropeller/pkg/compiler/errors/compiler_errors.go b/flytepropeller/pkg/compiler/errors/compiler_errors.go index 3f3d780e89..b2e3796edd 100755 --- a/flytepropeller/pkg/compiler/errors/compiler_errors.go +++ b/flytepropeller/pkg/compiler/errors/compiler_errors.go @@ -221,10 +221,10 @@ func NewMismatchingVariablesErr(nodeID, fromVar, fromType, toVar, toType string) ) } -func NewInvalidLiteralTypeErr(nodeID, errMsg string) *CompileError { +func NewInvalidLiteralTypeErr(nodeID, inputVar string, err error) *CompileError { return newError( InvalidLiteralTypeError, - errMsg, + fmt.Sprintf("Failed to validate literal type for [%s] with err: %s", inputVar, err), nodeID, ) } diff --git a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go index 7063ad3f2e..21250bd28d 100644 --- a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go +++ b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go @@ -1,8 +1,6 @@ package k8s import ( - "fmt" - "k8s.io/apimachinery/pkg/util/sets" "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core" @@ -47,8 +45,7 @@ func validateInputs(nodeID common.NodeID, iface *core.TypedInterface, inputs cor err := validators.ValidateLiteralType(inputType) if err != nil { - errMsg := fmt.Sprintf("Failed to validate literal type for [%s] with err: %s", inputVar, err) - errs.Collect(errors.NewInvalidLiteralTypeErr(nodeID, errMsg)) + errs.Collect(errors.NewInvalidLiteralTypeErr(nodeID, inputVar, err)) continue }