You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/06/04 09:05:25 UTC

[GitHub] [kafka] jlprat commented on a change in pull request #10810: MINOR: Improve Kafka Streams JavaDocs with regard to record metadata

jlprat commented on a change in pull request #10810:
URL: https://github.com/apache/kafka/pull/10810#discussion_r645405935



##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/ProcessorContext.java
##########
@@ -158,32 +165,52 @@ Cancellable schedule(final Duration interval,
     void commit();
 
     /**
-     * Returns the topic name of the current input record; could be null if it is not
-     * available (for example, if this method is invoked from the punctuate call).
+     * Returns the topic name of the current input record; could be {@code null} if it is not

Review comment:
       My apologies if there is already a KIP, I couldn't fine one for it. But would it make sense to create new KIP to add a new method returning an `Optional<String>`?

##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/RecordContext.java
##########
@@ -17,39 +17,82 @@
 package org.apache.kafka.streams.processor;
 
 import org.apache.kafka.common.header.Headers;
+import org.apache.kafka.streams.kstream.ValueTransformerWithKeySupplier;
 
 /**
  * The context associated with the current record being processed by
  * an {@link Processor}
  */
 public interface RecordContext {
+
     /**
-     * @return  The offset of the original record received from Kafka;
-     *          could be -1 if it is not available
+     * Returns the topic name of the current input record; could be {@code null} if it is not

Review comment:
       As before, my apologies if there is already a KIP, I couldn't fine one for it. But would it make sense to create new KIP to add a new method returning an `Optional<String>`?

##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/ProcessorContext.java
##########
@@ -158,32 +165,52 @@ Cancellable schedule(final Duration interval,
     void commit();
 
     /**
-     * Returns the topic name of the current input record; could be null if it is not
-     * available (for example, if this method is invoked from the punctuate call).
+     * Returns the topic name of the current input record; could be {@code null} if it is not
+     * available.
+     *
+     * <p> For example, if this method is invoked within a punctuate callback, or while processing a
+     * record that was forwarded by a punctuation callback, the record won't have an associated topic.
+     * Another example is
+     * {@link org.apache.kafka.streams.kstream.KTable#transformValues(ValueTransformerWithKeySupplier, String...)}
+     * (and siblings), that do not always guarantee to provide a valid topic name, as they might be
+     * executed "out-of-band" due to some internal optimizations applied by the Kafka Streams DSL.
      *
      * @return the topic name
      */
     String topic();
 
     /**
-     * Returns the partition id of the current input record; could be -1 if it is not
-     * available (for example, if this method is invoked from the punctuate call).
+     * Returns the partition id of the current input record; could be {@code -1} if it is not
+     * available.
+     *
+     * <p> For example, if this method is invoked within a punctuate callback, or while processing a
+     * record that was forwarded by a punctuation callback the record won't have an associated partition id.
+     * Another example is
+     * {@link org.apache.kafka.streams.kstream.KTable#transformValues(ValueTransformerWithKeySupplier, String...)}
+     * (and siblings), that do not always guarantee to provide a valid partition id, as they might be
+     * executed "out-of-band" due to some internal optimizations applied by the Kafka Streams DSL.
      *
      * @return the partition id
      */
     int partition();
 
     /**
-     * Returns the offset of the current input record; could be -1 if it is not
-     * available (for example, if this method is invoked from the punctuate call).
+     * Returns the offset of the current input record; could be {@code -1} if it is not
+     * available.
+     *
+     * <p> For example, if this method is invoked within a punctuate callback, or while processing a
+     * record that was forwarded by a punctuation callback, the record won't have an associated offset.

Review comment:
       Same as previous point

##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/ProcessorContext.java
##########
@@ -158,32 +165,52 @@ Cancellable schedule(final Duration interval,
     void commit();
 
     /**
-     * Returns the topic name of the current input record; could be null if it is not
-     * available (for example, if this method is invoked from the punctuate call).
+     * Returns the topic name of the current input record; could be {@code null} if it is not
+     * available.
+     *
+     * <p> For example, if this method is invoked within a punctuate callback, or while processing a
+     * record that was forwarded by a punctuation callback, the record won't have an associated topic.
+     * Another example is
+     * {@link org.apache.kafka.streams.kstream.KTable#transformValues(ValueTransformerWithKeySupplier, String...)}
+     * (and siblings), that do not always guarantee to provide a valid topic name, as they might be
+     * executed "out-of-band" due to some internal optimizations applied by the Kafka Streams DSL.
      *
      * @return the topic name
      */
     String topic();
 
     /**
-     * Returns the partition id of the current input record; could be -1 if it is not
-     * available (for example, if this method is invoked from the punctuate call).
+     * Returns the partition id of the current input record; could be {@code -1} if it is not
+     * available.
+     *
+     * <p> For example, if this method is invoked within a punctuate callback, or while processing a
+     * record that was forwarded by a punctuation callback the record won't have an associated partition id.

Review comment:
       Unless I'm mistaken, in line 153, as similar thing is said, but different styles are used (here no `{@link ...}` is used).
   Probably is worth unifying both styles?

##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/api/ProcessorContext.java
##########
@@ -53,9 +53,9 @@
     TaskId taskId();
 
     /**
-     * The metadata of the source record, if is one. Processors may be invoked to
+     * The metadata of the current record if available. Processors may be invoked to

Review comment:
       ```suggestion
        * Returns the metadata of the current record if available. Processors may be invoked to
   ```




-- 
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:
users@infra.apache.org