Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/3711#issuecomment-67991815
Hi @ksakellis,
This PR has some nice additions and looks pretty good overall. In
particular, I like the addition of the `SparkListenerAdapter` class, since it's
a nice way of shielding Java users from binary incompatbility when we add
things to SparkListener (this isn't an issue for Scala implementors of
SparkListener, since they inherit default implementations from the trait).
Good call on making it non-abstract, by the way.
I have one main piece of feedback: I think we need to add these new events
to JSONProtocol / JSONProtocolSuite so that they're properly persisted in the
event log. I think there's a bunch of examples of this in the code, so this
should be fairly straightforward. You may end up having to write JSON
serializers for ExecutorInfo.
---
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]
