vvcephei commented on a change in pull request #10810:
URL: https://github.com/apache/kafka/pull/10810#discussion_r647659103
##########
File path:
streams/src/main/java/org/apache/kafka/streams/processor/api/RecordMetadata.java
##########
@@ -16,19 +16,51 @@
*/
package org.apache.kafka.streams.processor.api;
+import org.apache.kafka.streams.kstream.ValueTransformerWithKeySupplier;
+
public interface RecordMetadata {
/**
- * @return The topic of the original record received from Kafka
+ * Returns the topic name of the current input record; could be {@code
null} if it is not
+ * available.
Review comment:
Oh boy. My recollection is that we were trying to keep a null context as
a sentinel so that we would be able to return a “not present” Optional. But we
are also populating “dummy” contexts in a few places, which might defeat that
logic. You’d have to trace through the code to figure out whether or not this
can happen. We’d better hurry up and deprecate the old API by the feature
freeze so that we can simplify these code paths. Ultimately, I agree: we
shouldn’t need nullable members inside an Optional container.
In the mean time, I don’t think the warning is harmful. It might cause
people to insert null checks that we can’t prove are unnecessary right now, but
if someone wants to comb through the codebase to prove it, we can always update
the Java doc later to say “never null”.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
