Add name to ExistsDifferentStructureError message (#5507)

Signed-off-by: Kevin Su <[email protected]>
flyteorg · Jun 25, 2024 · 5cc7f58 · 5cc7f58
1 parent 5c70453
commit 5cc7f58
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 7 deletions.
diff --git a/flyteadmin/pkg/errors/errors.go b/flyteadmin/pkg/errors/errors.go
@@ -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 := "task with different structure already exists:\n"
+	errorMsg := fmt.Sprintf("%v task with different structure already exists:\n", request.Id.Name)
 	diff, _ := jsondiff.Compare(oldSpec, newSpec)
 	rdiff, _ := jsondiff.Compare(newSpec, oldSpec)
 	rs := compareJsons(diff, rdiff)
@@ -145,7 +145,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 := "workflow with different structure already exists:\n"
+	errorMsg := fmt.Sprintf("%v workflow with different structure already exists:\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
@@ -19,7 +19,7 @@ var identifier = core.Identifier{
 	ResourceType: core.ResourceType_TASK,
 	Project:      "testProj",
 	Domain:       "domain",
-	Name:         "name",
+	Name:         "t1",
 	Version:      "ver",
 }
 
@@ -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, "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:\n\t\t- /template/Target/Container/resources/requests/0/value: 150m -> 250m", s.Message())
 }
 
 func TestNewTaskExistsIdenticalStructureError(t *testing.T) {
@@ -160,6 +160,14 @@ func TestNewTaskExistsIdenticalStructureError(t *testing.T) {
 }
 
 func TestNewWorkflowExistsDifferentStructureError(t *testing.T) {
+	identifier = core.Identifier{
+		ResourceType: core.ResourceType_WORKFLOW,
+		Project:      "testProj",
+		Domain:       "domain",
+		Name:         "hello",
+		Version:      "ver",
+	}
+
 	req := &admin.WorkflowCreateRequest{
 		Id: &identifier,
 	}
@@ -217,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, "workflow with different structure already exists:\n\t\t- /primary/connections/upstream/bar: <nil> -> 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]] -> <nil>\n\t\t- /primary/template/nodes/0/id: foo -> bar", s.Message())
+	assert.Equal(t, "hello workflow with different structure already exists:\n\t\t- /primary/connections/upstream/bar: <nil> -> 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]] -> <nil>\n\t\t- /primary/template/nodes/0/id: foo -> bar", s.Message())
 
 	details, ok := s.Details()[0].(*admin.CreateWorkflowFailureReason)
 	assert.True(t, ok)

diff --git 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, "workflow with different structure already exists:\n\t\t- /primary/template/id: <nil> -> map[domain:domain name:name project:project resource_type:2 version:version]\n\t\t- /primary/template/interface/inputs: <nil> -> map[variables:map[foo:map[type:map[Type:map[Simple:3]]]]]\n\t\t- /primary/template/interface/outputs: <nil> -> map[variables:map[bar:map[type:map[Type:map[Simple:3]]]]]\n\t\t- /primary/template/nodes: <nil> -> [map[Target:<nil> id:node 1] map[Target:<nil> id:node 2]]")
+	assert.EqualError(t, err, "name workflow with different structure already exists:\n\t\t- /primary/template/id: <nil> -> map[domain:domain name:name project:project resource_type:2 version:version]\n\t\t- /primary/template/interface/inputs: <nil> -> map[variables:map[foo:map[type:map[Type:map[Simple:3]]]]]\n\t\t- /primary/template/interface/outputs: <nil> -> map[variables:map[bar:map[type:map[Type:map[Simple:3]]]]]\n\t\t- /primary/template/nodes: <nil> -> [map[Target:<nil> id:node 1] map[Target:<nil> 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, "workflow with different structure already exists:\n\t\t- /primary/template/id: <nil> -> map[domain:domain name:name project:project resource_type:2 version:version]\n\t\t- /primary/template/interface/inputs: <nil> -> map[variables:map[foo:map[type:map[Type:map[Simple:3]]]]]\n\t\t- /primary/template/interface/outputs: <nil> -> map[variables:map[bar:map[type:map[Type:map[Simple:3]]]]]\n\t\t- /primary/template/nodes: <nil> -> [map[Target:<nil> id:node 1] map[Target:<nil> id:node 2]]")
+	assert.EqualError(t, err, "name workflow with different structure already exists:\n\t\t- /primary/template/id: <nil> -> map[domain:domain name:name project:project resource_type:2 version:version]\n\t\t- /primary/template/interface/inputs: <nil> -> map[variables:map[foo:map[type:map[Type:map[Simple:3]]]]]\n\t\t- /primary/template/interface/outputs: <nil> -> map[variables:map[bar:map[type:map[Type:map[Simple:3]]]]]\n\t\t- /primary/template/nodes: <nil> -> [map[Target:<nil> id:node 1] map[Target:<nil> id:node 2]]")
 	flyteErr := err.(flyteErrors.FlyteAdminError)
 	assert.Equal(t, codes.InvalidArgument, flyteErr.Code())
 	assert.Nil(t, response)