rednaxelafx commented on PR #39763: URL: https://github.com/apache/spark/pull/39763#issuecomment-2231799756
This looks good to me as well (non-binding). Thanks a lot for reviving this improvement! Agreed that the historical Spark UI itself wouldn't be affected. The REST API will get some exposure to the change though. Hopefully there's no client out in the wild that relies on this redundant information. The `JsonProtocolOptions` part looks pragmatic and I'm okay with that. If people are really annoyed by that, there's always the path of creating a `JsonProtocol` class that wraps the `JsonProtocolOptions` and delegates all operations down to the existing companion object. I don't think we have to do there here. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
