Github user reggert commented on the pull request:
https://github.com/apache/spark/pull/9264#issuecomment-156868899
I'm not particularly happy about the (internal) API exposed by
`ComplexFutureAction`. Having callers instantiate the action and then mutate it
by calling the `run` and `submitJob` methods just seems sloppy and error-prone.
I think I would prefer that instead of having a `run` method that takes a
closure returning `Future[T]`, `ComplexFutureAction` should accept a
constructor parameter of type `JobSubmitter => Future[T]`, where `JobSubmitter`
would be a (Spark-private) trait providing the `submitJob` method. This would
make `ComplexFutureAction` more or less immutable after construction (except
for cancellation) and prevent someone from calling `run` or `submitJob` from
outside Spark and making a mess of things. This is a fairly major changes
introducing a new trait, so I will hold off implementing it until I get some
positive feedback about it.
Additionally, it seems like certain common aspects of `SimpleFutureAction`
and `ComplexFutureAction` (such as the `_cancellation` field and the base
implementation of `cancel`) could be pulled out into an implementation trait
(i.e., `FutureActionLike`) to avoid duplicating code.
Thoughts?
---
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]
