[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #14433: [Fix] Fix running task instance throught api gots failed

["ruanwenjun (via GitHub)" <gi...@apache.org>]

ruanwenjun commented on code in PR #14433:
URL: https://github.com/apache/dolphinscheduler/pull/14433#discussion_r1252849966


##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/execute/TaskExecutionContextFactory.java:
##########
@@ -121,33 +121,39 @@ public TaskExecutionContext createTaskExecutionContext(TaskInstance taskInstance
                         .orElse(null);
         setTaskResourceInfo(resources);
 
-        // TODO to be optimized
-        DataQualityTaskExecutionContext dataQualityTaskExecutionContext = null;
-        if (TASK_TYPE_DATA_QUALITY.equalsIgnoreCase(taskInstance.getTaskType())) {
-            dataQualityTaskExecutionContext = new DataQualityTaskExecutionContext();
-            setDataQualityTaskRelation(dataQualityTaskExecutionContext, taskInstance, workflowInstance.getTenantCode());
-        }
-
-        K8sTaskExecutionContext k8sTaskExecutionContext = setK8sTaskRelation(taskInstance);
-
         Map<String, Property> businessParamsMap = curingParamsService.preBuildBusinessParams(workflowInstance);
 
         AbstractParameters baseParam = taskPluginManager.getParameters(ParametersNode.builder()
                 .taskType(taskInstance.getTaskType()).taskParams(taskInstance.getTaskParams()).build());
         Map<String, Property> propertyMap =
                 curingParamsService.paramParsingPreparation(taskInstance, baseParam, workflowInstance);
-        return TaskExecutionContextBuilder.get()
+        TaskExecutionContext taskExecutionContext = TaskExecutionContextBuilder.get()
                 .buildWorkflowInstanceHost(masterConfig.getMasterAddress())
                 .buildTaskInstanceRelatedInfo(taskInstance)
                 .buildTaskDefinitionRelatedInfo(taskInstance.getTaskDefine())
                 .buildProcessInstanceRelatedInfo(taskInstance.getProcessInstance())
                 .buildProcessDefinitionRelatedInfo(taskInstance.getProcessDefine())
                 .buildResourceParametersInfo(resources)
-                .buildDataQualityTaskExecutionContext(dataQualityTaskExecutionContext)
-                .buildK8sTaskRelatedInfo(k8sTaskExecutionContext)
                 .buildBusinessParamsMap(businessParamsMap)
                 .buildParamInfo(propertyMap)
                 .create();
+
+        buildTaskExecutionContext(taskExecutionContext, taskInstance, workflowInstance.getTenantCode());
+        return taskExecutionContext;
+    }
+
+    public void buildTaskExecutionContext(TaskExecutionContext taskExecutionContext, TaskInstance taskInstance,

Review Comment:
   This method will cause confusion, this is a side-effect method, and we don't know what will happen.
   
   It's better to split to `setDataQualityTaskExecutionContext` and `setK8sTaskRelatedInfo`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org