zhtaoxiang commented on code in PR #10132: URL: https://github.com/apache/pinot/pull/10132#discussion_r1073162110
########## pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java: ########## @@ -451,6 +455,48 @@ public String getSubtaskProgress(@Context HttpHeaders httpHeaders, } } + @GET + @Path("/tasks/subtask/state/{subTaskState}/progress") Review Comment: Thanks for the suggestions! I agree with you that the existing pattern is to have `taskType` or `taskName` path parameter and we are introducing another pattern here. Two other choices I have considered 1. As you mentioned, a single API `GET /tasks/subtask/progress` with all different filters (task name, subtask name, subtask state, minion worker ids) as query params. The problems are (1) not all filter combinations are valid, e.g., without knowing which tasks/subtasks are running on which minion workers, it's hard to provide a valid combination of (task name/subtask name, minion worker ids) (2) it's too generic, so it may be hard for users to understand how to use it correctly. (i.e., what to expect with different combinations.) (3) it's too generic, so it may also be hard for us to get it right (e.g., should we also task progress from task queue if a task is not assigned to a minion worker?) Personally, I prefer specific APIs to serve specific purpose. This API is doing one thing: get task process from minion workers. 2. Use the API path `GET /tasks/subtask/progress` with `sub task state and minion worker id filers`. The name is misleading. The problem is that users may think that this API is a super set of the exiting `GET /tasks/subtask/{taskName}/progress`. Personally, I prefer to go with option 2 if we can come up with a better API path. What do you think? -- 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-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org