[GitHub] [kafka] mjsax commented on a change in pull request #9744: KAFKA-10062: Add a method to retrieve the current timestamp as known by the Streams app

[GitBox <gi...@apache.org>]

mjsax commented on a change in pull request #9744:
URL: https://github.com/apache/kafka/pull/9744#discussion_r586727033



##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/ProcessorContext.java
##########
@@ -289,4 +291,33 @@ Cancellable schedule(final Duration interval,
      */
     Map<String, Object> appConfigsWithPrefix(final String prefix);
 
+    /**
+     * Return the current system timestamp (also called wall-clock time) in milliseconds.
+     *
+     * <p>
+     * Note: this method returns the internally cached system timestamp from the Kafka Stream runtime.
+     * Thus, it may return a different value compared to {@code System.currentTimeMillis()} .
+     * <p>

Review comment:
       nit. can be removed

##########
File path: streams/test-utils/src/main/java/org/apache/kafka/streams/processor/MockProcessorContext.java
##########
@@ -353,11 +371,19 @@ public void setHeaders(final Headers headers) {
      * The context exposes this metadata for use in the processor. Normally, they are set by the Kafka Streams framework,
      * but for the purpose of driving unit tests, you can set it directly. Setting this attribute doesn't affect the others.
      *
-     * @param timestamp A record timestamp
+     * @param recordTimestamp A record timestamp
      */
     @SuppressWarnings({"WeakerAccess", "unused"})
-    public void setTimestamp(final long timestamp) {
-        this.timestamp = timestamp;
+    public void setRecordTimestamp(final long recordTimestamp) {
+        this.recordTimestamp = recordTimestamp;
+    }
+
+    public void setCurrentSystemTimeMs(final long currentSystemTimeMs) {
+        this.currentSystemTimeMs = currentSystemTimeMs;
+    }
+
+    public void setCurrentStreamTimeMs(final long currentStreamTimeMs) {
+        this.currentStreamTimeMs = currentStreamTimeMs;

Review comment:
       This class is public API, so we cannot remove `setTimestamp` but can only deprecate it.
   
   We also need to update the KIP to mention the deprecation and the newly added methods.




----------------------------------------------------------------
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