suneet-s commented on code in PR #14771:
URL: https://github.com/apache/druid/pull/14771#discussion_r1295109496
##########
extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/KubernetesPeonLifecycle.java:
##########
@@ -117,7 +117,7 @@ protected KubernetesPeonLifecycle(
* @return
* @throws IllegalStateException
*/
- protected synchronized TaskStatus run(Job job, long launchTimeout, long
timeout) throws IllegalStateException
+ protected synchronized TaskStatus run(Job job, Task task, long
launchTimeout, long timeout) throws IllegalStateException
Review Comment:
This is a bit of a strange function definition, as the PeonLifecycle is
scoped to a `task`. Passing in both a task and a job is confusing, because they
should be to the same task. How about a function definition like this, and has
the constructor accept an `adapter` so that the adapter can be used in this
method
```suggestion
protected synchronized TaskStatus run(long launchTimeout, long timeout)
throws IllegalStateException
```
--
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]
