[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #13070: [Feature][API] New restful API for taskInstance

[GitBox <gi...@apache.org>]

zhongjiajie commented on code in PR #13070:
URL: https://github.com/apache/dolphinscheduler/pull/13070#discussion_r1039374034


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/TaskInstanceV2Controller.java:
##########
@@ -127,4 +174,27 @@ public TaskInstanceSuccessResponse forceTaskSuccess(@Parameter(hidden = true) @R
         Result result = taskInstanceService.forceTaskSuccess(loginUser, projectCode, id);
         return new TaskInstanceSuccessResponse(result);
     }
+
+    /**
+     * query taskInstance by taskInstanceCode
+     *
+     * @param loginUser   login user
+     * @param projectCode project code
+     * @param taskCode          task code
+     * @return the result code and msg
+     */
+    @Operation(summary = "queryOneTaskInstance", description = "QUERY_ONE_TASK_INSTANCE")
+    @Parameters({
+            @Parameter(name = "taskCode", description = "TASK_INSTANCE_CODE", required = true, schema = @Schema(implementation = Long.class), example = "1234567890")
+    })
+    @PostMapping(value = "/{taskCode}", consumes = {"application/json"})

Review Comment:
   I think we should use id instead of task code to get one single task instance 



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskInstanceServiceImpl.java:
##########
@@ -306,4 +306,31 @@ public Result stopTask(User loginUser, long projectCode, Integer taskInstanceId)
 
         return result;
     }
+
+    @Override
+    public Result queryTaskInstanceByCode(User loginUser, long projectCode, Long taskCode) {
+        Result result = new Result();
+
+        Project project = projectMapper.queryByCode(projectCode);
+        // check user access for project
+        Map<String, Object> checkResult =
+                projectService.checkProjectAndAuth(loginUser, project, projectCode, FORCED_SUCCESS);
+        Status status = (Status) checkResult.get(Constants.STATUS);
+        if (status != Status.SUCCESS) {
+            putMsg(result, status);
+            return result;
+        }
+        TaskInstance taskInstance = taskInstanceMapper.selectByCode(taskCode);
+        if (taskInstance == null) {
+            logger.error("Task definition can not be found, projectCode:{}, taskInstanceCode:{}.", projectCode,
+                    taskCode);

Review Comment:
   we can throw server exception here directly 



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskInstanceServiceImpl.java:
##########
@@ -306,4 +306,31 @@ public Result stopTask(User loginUser, long projectCode, Integer taskInstanceId)
 
         return result;
     }
+
+    @Override
+    public Result queryTaskInstanceByCode(User loginUser, long projectCode, Long taskCode) {

Review Comment:
   Yeah, we should return the entity instead of  Result object, the pros is when we have cli or API, they could use the return entity directly instead of by parsing the result object
   I think the results object should be wrapper by controller.



##########
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TaskInstanceMapper.java:
##########
@@ -154,5 +154,7 @@ IPage<TaskInstance> queryStreamTaskInstanceListPaging(IPage<TaskInstance> page,
     List<TaskInstance> loadAllInfosNoRelease(@Param("processInstanceId") int processInstanceId,
                                              @Param("status") int status);
 
+    TaskInstance selectByCode(Long taskCode);

Review Comment:
   So by id indeed, and will return one single record if we use id



-- 
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