Trogdor: Add Task State filter to /coordinator/tasks endpoint#5907
Trogdor: Add Task State filter to /coordinator/tasks endpoint#5907stanislavkozlovski wants to merge 7 commits intoapache:trunkfrom stanislavkozlovski:KAFKA-7619-trogdor-task-filter-state
Conversation
|
Can you rebase this change? |
|
Rebased |
|
Retest this please |
|
Looks good overall. I left a comment about types. |
Removes the old ManagedTaskState enum
| this.lastStartMs = Math.max(0, lastStartMs); | ||
| this.firstEndMs = Math.max(0, firstEndMs); | ||
| this.lastEndMs = Math.max(0, lastEndMs); | ||
| this.state = state; |
There was a problem hiding this comment.
Check for state == null and set this to Optional.None in that case
|
It looks like there is a findbugs warning that needs to be fixed. Also there is a null check that needs to be added. LGTM after that is done |
…o check if a given task is valid
| System.out.println("Got coordinator tasks: " + | ||
| JsonUtil.toPrettyJsonString(client.tasks( | ||
| new TasksRequest(null, 0, 0, 0, 0)))); | ||
| new TasksRequest(null, 0, 0, 0, 0, Optional.empty())))); |
There was a problem hiding this comment.
Can you file a follow-on JIRA for adding support for specifying the task state to show_tasks?
Then the user could run something like:
./bin/trogdor.sh client --show-tasks --state DONE
Alternately, we could add support in this pull request if you want.
There was a problem hiding this comment.
I decided we should open a JIRA since it would be good if we supported every argument for --show-tasks (taskids, start/end ms)
There was a problem hiding this comment.
|
|
||
| import com.fasterxml.jackson.annotation.JsonCreator; | ||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
| import org.apache.kafka.trogdor.coordinator.TaskManager; |
|
I merged this to trunk, after adding the missing suppression for an |
Reviewers: Colin McCabe <cmccabe@apache.org>
…#5907) Reviewers: Colin McCabe <cmccabe@apache.org>
2 participants
Also ensures that the
/tasks/{taskId}endpoint returns a semantically-correct 404 status code. It would previously return 500 since theNotFoundExceptionwas not handled anywhere.cc @cmccabe