HeartSaVioR commented on code in PR #41540:
URL: https://github.com/apache/spark/pull/41540#discussion_r1227475761
##########
sql/core/src/main/scala/org/apache/spark/sql/streaming/StreamingQueryListener.scala:
##########
@@ -123,7 +128,17 @@ object StreamingQueryListener {
val id: UUID,
val runId: UUID,
val name: String,
- val timestamp: String) extends Event
+ val timestamp: String) extends Event {
+
+ def json: String = compact(render(jsonValue))
+
+ private def jsonValue: JValue = {
Review Comment:
The test code you are referring to does not cover the case for version
compatibility. JsonProtocol is actually purposed to handle backward-and-forward
compatibility for events, but looks like we don't do it for SQL related events
since JsonProtocol does not have a visibility for SQL module. Maybe we need to
have another one for SQL when we figure out necessity of ensuring
backward-and-forward compatibility.
That's the context. Regarding this change, even if we use Jackson here,
underlying is still using json4s (see how StreamingQueryProgress is covered)
hence it becomes a mix-up. I'd like to see the entire thing to be replaced with
Jackson if we want to do that, for consistency.
Could we allow json4s be used for this PR, and file a follow up JIRA ticket
to replace entire part of streaming query listener events to use Jackson?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
