uros-b commented on code in PR #58467:
URL: https://github.com/apache/spark/pull/58467#discussion_r3914118977
##########
core/src/main/scala/org/apache/spark/status/protobuf/KVStoreProtobufSerializer.scala:
##########
@@ -51,6 +54,14 @@ private[spark] object KVStoreProtobufSerializer {
}.toMap
}
- def getSerializer(klass: Class[_]): Option[ProtobufSerDe[Any]] =
- serializerMap.get(klass)
+ private[this] val missedClasses = ConcurrentHashMap.newKeySet[Class[_]]()
+
+ def getSerializer(klass: Class[_]): Option[ProtobufSerDe[Any]] = {
Review Comment:
Please take a look at KVStoreProtobufSerializerSuite. No test covers the new
warning behavior. The PR description asserts "Existing tests" suffice, but both
KVStoreProtobufSerializerSuite suites (core and sql/core) exercise only
known-registered classes and never reach the serializer.isEmpty branch. A test
using withLogAppender should be added to assert (a) a warning IS logged on the
first call with an unregistered class, and (b) is NOT logged again on a
repeated call for the same class (the deduplication invariant). Without this,
the "warn exactly once" semantics are entirely untested.
--
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]
