Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/3614#issuecomment-66543595
+1; this looks like a good refactoring. I like how the new
`ExecutorAllocationManager` constructor makes its dependencies more explicit,
which should help us to avoid an entire category of "implicit initialization
order dependencies" bugs that we've hit in the past with classes that accept
partially-constructed SparkContexts in their constructors.
I think the mixin trait shouldn't be an issue here; I think that the
problems that I ran into with this pattern were related to the closure cleaner
and additional levels of `$outer` indirection, but that shouldn't be an issue
here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
