cadonna merged PR #15601:
URL: https://github.com/apache/kafka/pull/15601
--
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:
raminqaf commented on PR #15601:
URL: https://github.com/apache/kafka/pull/15601#issuecomment-2135147503
> @raminqaf Could you please rebase this PR on current trunk to get the new
build setup? We need to restart the builds because one was red.
Done!
--
This is an automated
cadonna commented on PR #15601:
URL: https://github.com/apache/kafka/pull/15601#issuecomment-2135043687
@raminqaf Could you please rebase this PR on current trunk to get the new
build setup? We need to restart the builds because one was red.
--
This is an automated message from the
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1607786468
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -187,17 +157,25 @@ public void process(final Record record) {
cadonna commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1605020116
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamLeftJoin.java:
##
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation
cadonna commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1604937635
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -187,17 +157,25 @@ public void process(final Record record) {
gharris1727 commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1600269458
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -43,7 +42,7 @@
import static
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1600077088
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -43,7 +42,7 @@
import static
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1600088096
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -43,7 +42,7 @@
import static
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1600088096
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -43,7 +42,7 @@
import static
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1600088096
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -43,7 +42,7 @@
import static
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1600076976
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamLeftJoin.java:
##
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software
cadonna commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r166724
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -43,7 +42,7 @@
import static
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1593500243
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -221,41 +199,28 @@ private void emitNonJoinedOuterRecords(
ableegoldman commented on PR #15601:
URL: https://github.com/apache/kafka/pull/15601#issuecomment-2099136409
Thanks Greg. If no one's had time to look at this by next week, we'll assign
a reviewer during the next Kafka Streams hangout
--
This is an automated message from the Apache Git
gharris1727 commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1592924655
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -187,17 +157,25 @@ public void process(final Record record) {
raminqaf commented on PR #15601:
URL: https://github.com/apache/kafka/pull/15601#issuecomment-2098817942
@gharris1727 Thanks for the feedback! I reverted all the changes you
requested and reverted a couple of other indentation problems that caused a
diff. I can even go further and revert &
gharris1727 commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1591293880
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -184,20 +146,34 @@ public void process(final Record record) {
raminqaf commented on PR #15601:
URL: https://github.com/apache/kafka/pull/15601#issuecomment-2084636567
@gharris1727 Thanks for the positive feedback! I am happy that you like the
changes! I addressed all your nit reviews and refactored the logic of the
non-final boolean variables (i.e.,
gharris1727 commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1583698116
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -240,27 +185,33 @@ private void emitNonJoinedOuterRecords(
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1563938474
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
Review Comment:
No more un safe type casting to be found in this
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1563929782
##
streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java:
##
@@ -63,21 +63,6 @@ public static LeftOrRightValue
makeRightValue(final
gharris1727 commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1554016667
##
streams/src/main/java/org/apache/kafka/streams/state/internals/TimestampedKeyAndJoinSide.java:
##
@@ -33,28 +34,36 @@
public class TimestampedKeyAndJoinSide {
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1545788943
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamRightJoin.java:
##
@@ -0,0 +1,221 @@
+/*
+ * Licensed to the Apache Software
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1545727392
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -16,276 +16,98 @@
*/
package
mcmmining commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1545728831
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamLeftJoin.java:
##
@@ -0,0 +1,201 @@
+/*
+ * Licensed to the Apache Software
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1545727392
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java:
##
@@ -16,276 +16,98 @@
*/
package
raminqaf commented on PR #15601:
URL: https://github.com/apache/kafka/pull/15601#issuecomment-2028756399
@gharris1727 I have broken down the KStreamKstreamJoin class into two
classes. For now, I just moved the code (+the fix in #15510) to see if all the
tests pass and if I am going in the
gharris1727 commented on PR #15601:
URL: https://github.com/apache/kafka/pull/15601#issuecomment-2027729205
This PR looks like it'll have merge conflicts with #15510. Since that is
fixing a bug it should probably have higher priority than this refactor, can
you rebase on top of their
gharris1727 commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1544779363
##
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamImplJoin.java:
##
@@ -41,6 +41,7 @@
import java.util.Optional;
import java.util.Set;
gharris1727 commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1544787082
##
streams/src/main/java/org/apache/kafka/streams/state/internals/JoinSide.java:
##
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1544749545
##
streams/src/main/java/org/apache/kafka/streams/state/internals/JoinSide.java:
##
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1544749545
##
streams/src/main/java/org/apache/kafka/streams/state/internals/JoinSide.java:
##
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1544749273
##
streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java:
##
Review Comment:
@gharris1727 Thanks for the fast review! I have
gharris1727 commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1540138631
##
streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java:
##
@@ -39,45 +39,6 @@ private LeftOrRightValue(final V1 leftValue, final
gharris1727 commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1540136195
##
streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java:
##
Review Comment:
I assumed that this would be necessary. I think
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1539078322
##
streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java:
##
Review Comment:
Retroperspectivly, we can refactor this class and
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1539078322
##
streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java:
##
Review Comment:
Retroperspectivly, we can refactor this class and
raminqaf commented on code in PR #15601:
URL: https://github.com/apache/kafka/pull/15601#discussion_r1539078322
##
streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java:
##
Review Comment:
Retroperspectivly, we can refactor this class and
raminqaf opened a new pull request, #15601:
URL: https://github.com/apache/kafka/pull/15601
Description
The introduced changes provide a cleaner definition of the join side in
KStreamKStreamJoin. Before, this was done by using a Boolean flag, which led to
returning a raw
40 matches
Mail list logo
