[jira] [Commented] (KAFKA-5233) Changes to punctuate semantics (KIP-138)
[ https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16352966#comment-16352966 ] ASF GitHub Bot commented on KAFKA-5233: --- guozhangwang closed pull request #3678: KAFKA-5233 follow up URL: https://github.com/apache/kafka/pull/3678 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamTransformTest.java b/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamTransformTest.java index d191891afe9..5aa7c7d94bb 100644 --- a/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamTransformTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamTransformTest.java @@ -24,6 +24,7 @@ import org.apache.kafka.streams.kstream.Transformer; import org.apache.kafka.streams.kstream.TransformerSupplier; import org.apache.kafka.streams.processor.ProcessorContext; +import org.apache.kafka.streams.processor.Punctuator; import org.apache.kafka.test.KStreamTestDriver; import org.apache.kafka.test.MockProcessorSupplier; import org.junit.Rule; @@ -39,38 +40,73 @@ @Rule public final KStreamTestDriver driver = new KStreamTestDriver(); +private Punctuator punctuator; + +private final TransformerSupplier> transformerSupplier = +new TransformerSupplier>() { +public Transformer> get() { +return new Transformer>() { + +private int total = 0; + +@Override +public void init(final ProcessorContext context) { +punctuator = new Punctuator() { +@Override +public void punctuate(long timestamp) { +context.forward(-1, (int) timestamp); +} +}; +} + +@Override +public KeyValue transform(Number key, Number value) { +total += value.intValue(); +return KeyValue.pair(key.intValue() * 2, total); +} + +@Override +public KeyValue punctuate(long timestamp) { +return KeyValue.pair(-1, (int) timestamp); +} + +@Override +public void close() { +} +}; +} +}; + @Test public void testTransform() { StreamsBuilder builder = new StreamsBuilder(); -TransformerSupplier> transformerSupplier = -new TransformerSupplier>() { -public Transformer> get() { -return new Transformer>() { +final int[] expectedKeys = {1, 10, 100, 1000}; + +MockProcessorSupplier processor = new MockProcessorSupplier<>(); +KStream stream = builder.stream(intSerde, intSerde, topicName); +stream.transform(transformerSupplier).process(processor); + +driver.setUp(builder); +for (int expectedKey : expectedKeys) { +driver.process(topicName, expectedKey, expectedKey * 10); +} -private int total = 0; +driver.punctuate(2, punctuator); +driver.punctuate(3, punctuator); -@Override -public void init(ProcessorContext context) { -} +assertEquals(6, processor.processed.size()); -@Override -public KeyValue transform(Number key, Number value) { -total += value.intValue(); -return KeyValue.pair(key.intValue() * 2, total); -} +String[] expected = {"2:10", "20:110", "200:1110", "2000:0", "-1:2", "-1:3"}; -@Override -public KeyValue punctuate(long timestamp) { -return KeyValue.pair(-1, (int) timestamp); -} +for (int i = 0; i < expected.length; i++) { +assertEquals(expected[i], processor.processed.get(i)); +} +} -@Override -public void close() { -} -}; -} -}; +@Test @Deprecated +public void testTransformWithDeprecatedPunctuate() { +StreamsBuilder builder = new StreamsBuilder(); final int[] expectedKeys = {1, 10, 100, 1000}; @@ -83,8 +119,8 @@
[jira] [Commented] (KAFKA-5233) Changes to punctuate semantics (KIP-138)
[ https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16140959#comment-16140959 ] Guozhang Wang commented on KAFKA-5233: -- [~mihbor] I have another pr for other related doc changes: https://github.com/apache/kafka/pull/3732 and among them I have a minor proposal to rename SYSTEM_TIME to WALL_CLOCK_TIME before the 1.0.0 release, LMK your thoughts. > Changes to punctuate semantics (KIP-138) > > > Key: KAFKA-5233 > URL: https://issues.apache.org/jira/browse/KAFKA-5233 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Michal Borowiecki >Assignee: Michal Borowiecki > Labels: kip > Fix For: 1.0.0 > > > This ticket is to track implementation of > [KIP-138: Change punctuate > semantics|https://cwiki.apache.org/confluence/display/KAFKA/KIP-138%3A+Change+punctuate+semantics] -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (KAFKA-5233) Changes to punctuate semantics (KIP-138)
[ https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16129646#comment-16129646 ] Guozhang Wang commented on KAFKA-5233: -- Thanks [~mihbor]! > Changes to punctuate semantics (KIP-138) > > > Key: KAFKA-5233 > URL: https://issues.apache.org/jira/browse/KAFKA-5233 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Michal Borowiecki >Assignee: Michal Borowiecki > Labels: kip > Fix For: 1.0.0 > > > This ticket is to track implementation of > [KIP-138: Change punctuate > semantics|https://cwiki.apache.org/confluence/display/KAFKA/KIP-138%3A+Change+punctuate+semantics] -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (KAFKA-5233) Changes to punctuate semantics (KIP-138)
[ https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16129556#comment-16129556 ] Michal Borowiecki commented on KAFKA-5233: -- Hi [~guozhang], I had a look at KStreamTestDriver and the punctuate method defined there is only ever called from the KStreamTransformTest to trigger the deprecated punctuate method on the Transformer interface. The test validates that records returned from Transformer.punctuate are correctly forwarded and processed by the downstream Processor. I think there is value it preserving this test until the deprecated method is removed. However, I've added a new equivalent test to check that calling context.forward from within the Punctuator callback achieves the same result. I renamed KStreamTestDriver's punctuate to punctuateDeprecated and annotated it as such to make it clearer it's only left to test deprecated functionality so that it's not accidentally used for new developments. > Changes to punctuate semantics (KIP-138) > > > Key: KAFKA-5233 > URL: https://issues.apache.org/jira/browse/KAFKA-5233 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Michal Borowiecki >Assignee: Michal Borowiecki > Labels: kip > Fix For: 1.0.0 > > > This ticket is to track implementation of > [KIP-138: Change punctuate > semantics|https://cwiki.apache.org/confluence/display/KAFKA/KIP-138%3A+Change+punctuate+semantics] -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (KAFKA-5233) Changes to punctuate semantics (KIP-138)
[ https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16129533#comment-16129533 ] ASF GitHub Bot commented on KAFKA-5233: --- GitHub user mihbor opened a pull request: https://github.com/apache/kafka/pull/3678 KAFKA-5233 follow up You can merge this pull request into a Git repository by running: $ git pull https://github.com/mihbor/kafka trunk Alternatively you can review and apply these changes as the patch at: https://github.com/apache/kafka/pull/3678.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3678 commit f6da0be924c6fb657d532ea4cf2b1877354e00d1 Author: Michal Borowiecki Date: 2017-08-16T22:21:33Z KAFKA-5233 follow up > Changes to punctuate semantics (KIP-138) > > > Key: KAFKA-5233 > URL: https://issues.apache.org/jira/browse/KAFKA-5233 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Michal Borowiecki >Assignee: Michal Borowiecki > Labels: kip > Fix For: 1.0.0 > > > This ticket is to track implementation of > [KIP-138: Change punctuate > semantics|https://cwiki.apache.org/confluence/display/KAFKA/KIP-138%3A+Change+punctuate+semantics] -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (KAFKA-5233) Changes to punctuate semantics (KIP-138)
[ https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16107653#comment-16107653 ] Guozhang Wang commented on KAFKA-5233: -- I agree with both of your regarding keeping the unit tests for the deprecated-but-not-removed functionalities. What I was mentioning is that for some unit test util functions like {{KStreamTestDriver}} it should be updated with the new APIs as it is not for the test coverage, but for helping with the unit tests and hence should be called with the latest stable APIs. > Changes to punctuate semantics (KIP-138) > > > Key: KAFKA-5233 > URL: https://issues.apache.org/jira/browse/KAFKA-5233 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Michal Borowiecki >Assignee: Michal Borowiecki > Labels: kip > Fix For: 1.0.0 > > > This ticket is to track implementation of > [KIP-138: Change punctuate > semantics|https://cwiki.apache.org/confluence/display/KAFKA/KIP-138%3A+Change+punctuate+semantics] -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (KAFKA-5233) Changes to punctuate semantics (KIP-138)
[ https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16106404#comment-16106404 ] Matthias J. Sax commented on KAFKA-5233: I agree with [~mihbor] that we might want to keep the old tests. That's why I just added the {{@suppress}} annotation in the other PR you mentioned. At the point we decide to really remove deprecated APIs completely, all old code will go away anyway. > Changes to punctuate semantics (KIP-138) > > > Key: KAFKA-5233 > URL: https://issues.apache.org/jira/browse/KAFKA-5233 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Michal Borowiecki >Assignee: Michal Borowiecki > Labels: kip > Fix For: 1.0.0 > > > This ticket is to track implementation of > [KIP-138: Change punctuate > semantics|https://cwiki.apache.org/confluence/display/KAFKA/KIP-138%3A+Change+punctuate+semantics] -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (KAFKA-5233) Changes to punctuate semantics (KIP-138)
[ https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16106084#comment-16106084 ] Michal Borowiecki commented on KAFKA-5233: -- Yes, my line of thinking was that although deprecated, the old punctuate still has to work so it would be better not to lose test coverage for it. I'll raise a ticket to eventually remove the deprecated punctuate method and that feels to me like the natural time to also remove associated unit tests. On the other hand, if you found any unit tests that I had missed, let me know which tests those are and I'll add equivalent tests for the new punctuate method. > Changes to punctuate semantics (KIP-138) > > > Key: KAFKA-5233 > URL: https://issues.apache.org/jira/browse/KAFKA-5233 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Michal Borowiecki >Assignee: Michal Borowiecki > Labels: kip > Fix For: 1.0.0 > > > This ticket is to track implementation of > [KIP-138: Change punctuate > semantics|https://cwiki.apache.org/confluence/display/KAFKA/KIP-138%3A+Change+punctuate+semantics] -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (KAFKA-5233) Changes to punctuate semantics (KIP-138)
[ https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105814#comment-16105814 ] Guozhang Wang commented on KAFKA-5233: -- [~mihbor] While reviewing some other PRs I realized that there are some places in unit tests that are still referring to the deprecated `punctuate` function. For example in {{KStreamTestDriver}}. Could you file a follow-up PR to clear them up as well? cc [~damianguy] [~mjsax] > Changes to punctuate semantics (KIP-138) > > > Key: KAFKA-5233 > URL: https://issues.apache.org/jira/browse/KAFKA-5233 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Michal Borowiecki >Assignee: Michal Borowiecki > Labels: kip > Fix For: 1.0.0 > > > This ticket is to track implementation of > [KIP-138: Change punctuate > semantics|https://cwiki.apache.org/confluence/display/KAFKA/KIP-138%3A+Change+punctuate+semantics] -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (KAFKA-5233) Changes to punctuate semantics (KIP-138)
[ https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16066296#comment-16066296 ] ASF GitHub Bot commented on KAFKA-5233: --- Github user asfgit closed the pull request at: https://github.com/apache/kafka/pull/3055 > Changes to punctuate semantics (KIP-138) > > > Key: KAFKA-5233 > URL: https://issues.apache.org/jira/browse/KAFKA-5233 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Michal Borowiecki >Assignee: Michal Borowiecki > Labels: kip > Fix For: 0.11.1.0 > > > This ticket is to track implementation of > [KIP-138: Change punctuate > semantics|https://cwiki.apache.org/confluence/display/KAFKA/KIP-138%3A+Change+punctuate+semantics] -- This message was sent by Atlassian JIRA (v6.4.14#64029)