Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/21068#discussion_r185534467
--- Diff:
resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnAllocatorSuite.scala
---
@@ -354,7 +358,7 @@ class YarnAllocatorSuite extends SparkFunSuite with
Matchers with BeforeAndAfter
sparkConf.set("spark.yarn.executor.failuresValidityInterval", "100s")
val handler = createAllocator(4)
val clock = new ManualClock(0L)
- handler.setClock(clock)
+ handler.failureTracker.setClock(clock)
--- End diff --
I agree with Marcelo's earlier comment about eliminating `setClock`. Make
it an optional argument to `YarnAllocator`'s constructor, which then gets
pushed down. This is only reason you need to make it a `var`, and you could
just as easily do
```scala
val clock = new ManualClock(0L)
val handler = createAllocator(4, clock)
```
(I realize this is the way the old code was, just a good chance to clean it
up a bit.)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
