YongGang commented on PR #14643:
URL: https://github.com/apache/druid/pull/14643#issuecomment-1647233568
The problem of option 1 is the constructor of `KubernetesWorkItem` will call
the superclass' constructor as well. And even if we opt to change both
`KubernetesWorkItem` and `TaskRunnerWorkItem` class to add
`setResultIfRequired` method is still not ideal and error prone as developers
need to remember to call `setResultIfRequired` right after instance being
created.
```
public class KubernetesWorkItem extends TaskRunnerWorkItem
{
public KubernetesWorkItem(Task task, ListenableFuture<TaskStatus>
statusFuture)
{
super(task.getId(), statusFuture);
this.task = task;
}
```
In this case I prefer to revert last commit. Though synchronized on tasks in
all the operations seems overkill as it's already a ConcurrentHashMap but it
makes the class more thread-safe (in theory).
--
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]
