You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "mjsax (via GitHub)" <gi...@apache.org> on 2025/02/02 21:58:33 UTC

[PR] MINOR: fix NPE in KS `Topology` for new `AutoOffsetReset` [kafka]

mjsax opened a new pull request, #18780:
URL: https://github.com/apache/kafka/pull/18780

   Introduced via KIP-1106.
   
   Needs to be cherry-picked to `4.0` branch, to avoid shipping a new feature with this bug. \cc @dajac 


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: fix NPE in KS `Topology` for new `AutoOffsetReset` [kafka]

Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #18780:
URL: https://github.com/apache/kafka/pull/18780#issuecomment-2632569081

   Merged to `trunk` and cherry-picked to `4.0` branch.


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: fix NPE in KS `Topology` for new `AutoOffsetReset` [kafka]

Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #18780:
URL: https://github.com/apache/kafka/pull/18780#issuecomment-2629852948

   Java 17 passed.
   
   Java 23 had no test failures, but build did not pass. No idea what happened:
   ```
   76 PASSED ✅, 0 FAILED ❌, 0 FLAKY ⚠️ , 1 SKIPPED 🙈, 76 QUARANTINED 😷, and 0 errors.
   Failing this step because the tests timed out. Thread dumps were not archived, check logs in JUnit step.
   Error: Process completed with exit code 1.
   ```


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: fix NPE in KS `Topology` for new `AutoOffsetReset` [kafka]

Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on code in PR #18780:
URL: https://github.com/apache/kafka/pull/18780#discussion_r1938590363


##########
streams/src/test/java/org/apache/kafka/streams/TopologyTest.java:
##########
@@ -2338,7 +2338,6 @@ private TopologyDescription.Sink addSink(final String sinkName,
         return expectedSinkNode;
     }
 
-    @Deprecated // testing old PAPI

Review Comment:
   Code was already updated for new PAPI. Tag and comment are just outdated, so removing it.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: fix NPE in KS `Topology` for new `AutoOffsetReset` [kafka]

Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on code in PR #18780:
URL: https://github.com/apache/kafka/pull/18780#discussion_r1938590264


##########
streams/src/test/java/org/apache/kafka/streams/TopologyTest.java:
##########
@@ -2257,16 +2257,16 @@ private Topology topologyWithStaticTopicName() {
 
     private TopologyDescription.Source addSource(final String sourceName,
                                                  final String... sourceTopic) {
-        topology.addSource((Topology.AutoOffsetReset) null, sourceName, null, null, null, sourceTopic);
-        final StringBuilder allSourceTopics = new StringBuilder(sourceTopic[0]);
-        for (int i = 1; i < sourceTopic.length; ++i) {
-            allSourceTopics.append(", ").append(sourceTopic[i]);
-        }
+        topology.addSource((AutoOffsetReset) null, sourceName, null, null, null, sourceTopic);

Review Comment:
   While cleaning up the code, I found that we cast to the "old" `Topology.AutoOffsetReset` enum here. Changing it to use the new `AutoOffsetReset` class, `TopologyTest` broke exposing the bug.
   
   Everything else in this class is just side-cleanup.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: fix NPE in KS `Topology` for new `AutoOffsetReset` [kafka]

Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax merged PR #18780:
URL: https://github.com/apache/kafka/pull/18780


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org