diff --git a/flyteadmin/pkg/errors/errors.go b/flyteadmin/pkg/errors/errors.go index 78727a7305..0fd9542ba7 100644 --- a/flyteadmin/pkg/errors/errors.go +++ b/flyteadmin/pkg/errors/errors.go @@ -112,7 +112,7 @@ func NewIncompatibleClusterError(ctx context.Context, errorMsg, curCluster strin } func compareJsons(jsonArray1 jsondiff.Patch, jsonArray2 jsondiff.Patch) []string { - results := []string{} + var results []string map1 := make(map[string]jsondiff.Operation) for _, obj := range jsonArray1 { map1[obj.Path] = obj @@ -120,7 +120,7 @@ func compareJsons(jsonArray1 jsondiff.Patch, jsonArray2 jsondiff.Patch) []string for _, obj := range jsonArray2 { if val, ok := map1[obj.Path]; ok { - result := fmt.Sprintf("\t\t- %v: %v -> %v", obj.Path, obj.Value, val.Value) + result := fmt.Sprintf("%v:\n\t- %v \n\t+ %v", obj.Path, obj.Value, val.Value) results = append(results, result) } } @@ -128,7 +128,7 @@ func compareJsons(jsonArray1 jsondiff.Patch, jsonArray2 jsondiff.Patch) []string } func NewTaskExistsDifferentStructureError(ctx context.Context, request *admin.TaskCreateRequest, oldSpec *core.CompiledTask, newSpec *core.CompiledTask) FlyteAdminError { - errorMsg := fmt.Sprintf("%v task with different structure already exists:\n", request.Id.Name) + errorMsg := fmt.Sprintf("%v task with different structure already exists. (Please register a new version of the task):\n", request.Id.Name) diff, _ := jsondiff.Compare(oldSpec, newSpec) rdiff, _ := jsondiff.Compare(newSpec, oldSpec) rs := compareJsons(diff, rdiff) @@ -136,7 +136,6 @@ func NewTaskExistsDifferentStructureError(ctx context.Context, request *admin.Ta errorMsg += strings.Join(rs, "\n") return NewFlyteAdminErrorf(codes.InvalidArgument, errorMsg) - } func NewTaskExistsIdenticalStructureError(ctx context.Context, request *admin.TaskCreateRequest) FlyteAdminError { @@ -145,7 +144,7 @@ func NewTaskExistsIdenticalStructureError(ctx context.Context, request *admin.Ta } func NewWorkflowExistsDifferentStructureError(ctx context.Context, request *admin.WorkflowCreateRequest, oldSpec *core.CompiledWorkflowClosure, newSpec *core.CompiledWorkflowClosure) FlyteAdminError { - errorMsg := fmt.Sprintf("%v workflow with different structure already exists:\n", request.Id.Name) + errorMsg := fmt.Sprintf("%v workflow with different structure already exists. (Please register a new version of the workflow):\n", request.Id.Name) diff, _ := jsondiff.Compare(oldSpec, newSpec) rdiff, _ := jsondiff.Compare(newSpec, oldSpec) rs := compareJsons(diff, rdiff) @@ -183,7 +182,7 @@ func NewWorkflowExistsIdenticalStructureError(ctx context.Context, request *admi } func NewLaunchPlanExistsDifferentStructureError(ctx context.Context, request *admin.LaunchPlanCreateRequest, oldSpec *admin.LaunchPlanSpec, newSpec *admin.LaunchPlanSpec) FlyteAdminError { - errorMsg := "launch plan with different structure already exists:\n" + errorMsg := fmt.Sprintf("%v launch plan with different structure already exists. (Please register a new version of the launch plan):\n", request.Id.Name) diff, _ := jsondiff.Compare(oldSpec, newSpec) rdiff, _ := jsondiff.Compare(newSpec, oldSpec) rs := compareJsons(diff, rdiff) diff --git a/flyteadmin/pkg/errors/errors_test.go b/flyteadmin/pkg/errors/errors_test.go index daaa060340..18c76992b5 100644 --- a/flyteadmin/pkg/errors/errors_test.go +++ b/flyteadmin/pkg/errors/errors_test.go @@ -74,7 +74,7 @@ func TestJsonDifferHasDiffError(t *testing.T) { diff, _ := jsondiff.Compare(oldSpec, newSpec) rdiff, _ := jsondiff.Compare(newSpec, oldSpec) rs := compareJsons(diff, rdiff) - assert.Equal(t, "\t\t- /four: 4 -> 0", strings.Join(rs, "\n")) + assert.Equal(t, "/four:\n\t- 4 \n\t+ 0", strings.Join(rs, "\n")) } func TestJsonDifferNoDiffError(t *testing.T) { @@ -144,7 +144,7 @@ func TestNewTaskExistsDifferentStructureError(t *testing.T) { s, ok := status.FromError(statusErr) assert.True(t, ok) assert.Equal(t, codes.InvalidArgument, s.Code()) - assert.Equal(t, "t1 task with different structure already exists:\n\t\t- /template/Target/Container/resources/requests/0/value: 150m -> 250m", s.Message()) + assert.Equal(t, "t1 task with different structure already exists. (Please register a new version of the task):\n/template/Target/Container/resources/requests/0/value:\n\t- 150m \n\t+ 250m", s.Message()) } func TestNewTaskExistsIdenticalStructureError(t *testing.T) { @@ -160,7 +160,7 @@ func TestNewTaskExistsIdenticalStructureError(t *testing.T) { } func TestNewWorkflowExistsDifferentStructureError(t *testing.T) { - identifier = core.Identifier{ + identifier := core.Identifier{ ResourceType: core.ResourceType_WORKFLOW, Project: "testProj", Domain: "domain", @@ -225,7 +225,7 @@ func TestNewWorkflowExistsDifferentStructureError(t *testing.T) { s, ok := status.FromError(statusErr) assert.True(t, ok) assert.Equal(t, codes.InvalidArgument, s.Code()) - assert.Equal(t, "hello workflow with different structure already exists:\n\t\t- /primary/connections/upstream/bar: -> map[ids:[start-node]]\n\t\t- /primary/connections/upstream/end-node/ids/0: foo -> bar\n\t\t- /primary/connections/upstream/foo: map[ids:[start-node]] -> \n\t\t- /primary/template/nodes/0/id: foo -> bar", s.Message()) + assert.Equal(t, "hello workflow with different structure already exists. (Please register a new version of the workflow):\n/primary/connections/upstream/bar:\n\t- \n\t+ map[ids:[start-node]]\n/primary/connections/upstream/end-node/ids/0:\n\t- foo \n\t+ bar\n/primary/connections/upstream/foo:\n\t- map[ids:[start-node]] \n\t+ \n/primary/template/nodes/0/id:\n\t- foo \n\t+ bar", s.Message()) details, ok := s.Details()[0].(*admin.CreateWorkflowFailureReason) assert.True(t, ok) @@ -251,6 +251,11 @@ func TestNewWorkflowExistsIdenticalStructureError(t *testing.T) { } func TestNewLaunchPlanExistsDifferentStructureError(t *testing.T) { + identifier := core.Identifier{ + ResourceType: core.ResourceType_LAUNCH_PLAN, + Name: "lp_name", + } + req := &admin.LaunchPlanCreateRequest{ Id: &identifier, } @@ -284,7 +289,7 @@ func TestNewLaunchPlanExistsDifferentStructureError(t *testing.T) { s, ok := status.FromError(statusErr) assert.True(t, ok) assert.Equal(t, codes.InvalidArgument, s.Code()) - assert.Equal(t, "launch plan with different structure already exists:\n\t\t- /workflow_id/version: ver1 -> ver2", s.Message()) + assert.Equal(t, "lp_name launch plan with different structure already exists. (Please register a new version of the launch plan):\n/workflow_id/version:\n\t- ver1 \n\t+ ver2", s.Message()) } func TestNewLaunchPlanExistsIdenticalStructureError(t *testing.T) { diff --git a/flyteadmin/pkg/manager/impl/workflow_manager_test.go b/flyteadmin/pkg/manager/impl/workflow_manager_test.go index 60e0187e07..33626cae84 100644 --- a/flyteadmin/pkg/manager/impl/workflow_manager_test.go +++ b/flyteadmin/pkg/manager/impl/workflow_manager_test.go @@ -187,7 +187,7 @@ func TestCreateWorkflow_ExistingWorkflow(t *testing.T) { getMockWorkflowConfigProvider(), getMockWorkflowCompiler(), mockStorageClient, storagePrefix, mockScope.NewTestScope()) request := testutils.GetWorkflowRequest() response, err := workflowManager.CreateWorkflow(context.Background(), request) - assert.EqualError(t, err, "name workflow with different structure already exists:\n\t\t- /primary/template/id: -> map[domain:domain name:name project:project resource_type:2 version:version]\n\t\t- /primary/template/interface/inputs: -> map[variables:map[foo:map[type:map[Type:map[Simple:3]]]]]\n\t\t- /primary/template/interface/outputs: -> map[variables:map[bar:map[type:map[Type:map[Simple:3]]]]]\n\t\t- /primary/template/nodes: -> [map[Target: id:node 1] map[Target: id:node 2]]") + assert.EqualError(t, err, "name workflow with different structure already exists. (Please register a new version of the workflow):\n/primary/template/id:\n\t- \n\t+ map[domain:domain name:name project:project resource_type:2 version:version]\n/primary/template/interface/inputs:\n\t- \n\t+ map[variables:map[foo:map[type:map[Type:map[Simple:3]]]]]\n/primary/template/interface/outputs:\n\t- \n\t+ map[variables:map[bar:map[type:map[Type:map[Simple:3]]]]]\n/primary/template/nodes:\n\t- \n\t+ [map[Target: id:node 1] map[Target: id:node 2]]") assert.Nil(t, response) } @@ -205,7 +205,7 @@ func TestCreateWorkflow_ExistingWorkflow_Different(t *testing.T) { request := testutils.GetWorkflowRequest() response, err := workflowManager.CreateWorkflow(context.Background(), request) - assert.EqualError(t, err, "name workflow with different structure already exists:\n\t\t- /primary/template/id: -> map[domain:domain name:name project:project resource_type:2 version:version]\n\t\t- /primary/template/interface/inputs: -> map[variables:map[foo:map[type:map[Type:map[Simple:3]]]]]\n\t\t- /primary/template/interface/outputs: -> map[variables:map[bar:map[type:map[Type:map[Simple:3]]]]]\n\t\t- /primary/template/nodes: -> [map[Target: id:node 1] map[Target: id:node 2]]") + assert.EqualError(t, err, "name workflow with different structure already exists. (Please register a new version of the workflow):\n/primary/template/id:\n\t- \n\t+ map[domain:domain name:name project:project resource_type:2 version:version]\n/primary/template/interface/inputs:\n\t- \n\t+ map[variables:map[foo:map[type:map[Type:map[Simple:3]]]]]\n/primary/template/interface/outputs:\n\t- \n\t+ map[variables:map[bar:map[type:map[Type:map[Simple:3]]]]]\n/primary/template/nodes:\n\t- \n\t+ [map[Target: id:node 1] map[Target: id:node 2]]") flyteErr := err.(flyteErrors.FlyteAdminError) assert.Equal(t, codes.InvalidArgument, flyteErr.Code()) assert.Nil(t, response)