[jira] [Commented] (KAFKA-5233) Changes to punctuate semantics (KIP-138)

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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++) {
+

[jira] [Commented] (KAFKA-5233) Changes to punctuate semantics (KIP-138)

[Guozhang Wang (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[Guozhang Wang (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[Michal Borowiecki (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[Guozhang Wang (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[Matthias J. Sax (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[Michal Borowiecki (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[Guozhang Wang (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-5233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)