[GitHub] [kafka] vvcephei commented on a change in pull request #10421: KAFKA-12568: Remove deprecated APIs in KStream, KTable and Joined

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

vvcephei commented on a change in pull request #10421:
URL: https://github.com/apache/kafka/pull/10421#discussion_r609080989



##########
File path: streams/src/test/java/org/apache/kafka/streams/StreamsBuilderTest.java
##########
@@ -725,12 +725,11 @@ public void shouldUseSpecifiedNameForLeftJoinOperationBetweenKStreamAndKStream()
     }
 
     @Test
-    @Deprecated
     public void shouldUseGeneratedStoreNamesForLeftJoinOperationBetweenKStreamAndKStream() {
         final KStream<String, String> streamOne = builder.stream(STREAM_TOPIC);
         final KStream<String, String> streamTwo = builder.stream(STREAM_TOPIC_TWO);
 
-        streamOne.leftJoin(streamTwo, (value1, value2) -> value1, JoinWindows.of(Duration.ofHours(1)), Joined.as(STREAM_OPERATION_NAME));
+        streamOne.leftJoin(streamTwo, (value1, value2) -> value1, JoinWindows.of(Duration.ofHours(1)), StreamJoined.with(Serdes.String(), Serdes.String(), Serdes.String()).withName(STREAM_OPERATION_NAME));

Review comment:
       Oh, I see.
   
   The docs on `Joined.as` say: "Create an instance of {@code Joined} with base name for all components of the join, this may include any repartition topics created to complete the join"
   
   `StreamJoined#withName` says something similar: "Set the name to use for the join processor and the repartition topic(s) if required"
   
   Whereas `StreamJoined.as` says: "Creates a {@link StreamJoined} instance using the provided name for the state stores and hence the changelog topics for the join stores")
   
   So, it sounds like we realized that we'd accidentally forgotten to name the stores as part of `Joined.as` and `StreamJoined.withName` and patched it up in `StreamJoined.as`.
   
   But of course, we couldn't just fix it in-place, so we wound up with this wacky combination. I'm honestly not sure the best way to proceed. I doubt very many people are going to figure out what you figured out. Maybe we should just add a really verbose javadoc on `StreamJoined.as` explaining what they need to do if they're coming from `Joined.as`?




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