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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
