KAFKA-4672 fix source compatibility for lambda expressions by xvrl · Pull Request #2402 · apache/kafka

xvrl · 2017-01-18T22:16:56Z

Variance changes introduced in KIP-100 cause compilation failures with lambda expression in Java 8. To my knowledge this only affects the following method

KStreams.transform(TransformerSupplier<...>, String...)

prior to the changes it was possible to write:

streams.transform(MyTransformer::new)

where MyTransformer extends Transformer

After the changes the Java compiler is unable to infer correct return types for the lambda expressions. This change fixed this by reverting to invariant return types for transformer suppliers.

please cherry-pick into 0.10.2.x

enothereska · 2017-01-18T22:32:06Z

cc'ing @ijuma

asfbot · 2017-01-18T23:06:02Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/998/
Test FAILed (JDK 8 and Scala 2.11).

xvrl · 2017-01-18T23:27:11Z

retest this please

asfbot · 2017-01-18T23:40:08Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/996/
Test PASSed (JDK 8 and Scala 2.12).

ijuma · 2017-01-18T23:54:23Z

It would be good to explain the reasoning in the PR description for posterity.

asfbot · 2017-01-18T23:58:16Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/996/
Test FAILed (JDK 7 and Scala 2.10).

xvrl · 2017-01-19T00:01:18Z

retest this please

xvrl · 2017-01-19T00:12:20Z

@ijuma updated pr description

asfbot · 2017-01-19T00:50:25Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/1006/
Test PASSed (JDK 8 and Scala 2.11).

asfbot · 2017-01-19T00:50:48Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/1004/
Test PASSed (JDK 8 and Scala 2.12).

asfbot · 2017-01-19T00:56:04Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/1004/
Test PASSed (JDK 7 and Scala 2.10).

enothereska · 2017-01-19T09:11:51Z

cc'ing @dguy

dguy · 2017-01-19T09:20:32Z

I don't think this one needs to change. AFAICT it is only transform that has the issue

you are correct, fixed

enothereska · 2017-01-19T09:26:57Z

I think one issue is that we don't have tests for Java 8 handy.

asfbot · 2017-01-19T17:34:01Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/1026/
Test FAILed (JDK 7 and Scala 2.10).

dguy

LGTM

asfbot · 2017-01-19T17:35:46Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/1028/
Test PASSed (JDK 8 and Scala 2.11).

xvrl · 2017-01-19T18:09:05Z

retest this please

enothereska · 2017-01-19T18:21:37Z

Ok, LGTM. Thanks!

asfbot · 2017-01-19T18:58:04Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/1031/
Test PASSed (JDK 8 and Scala 2.11).

asfbot · 2017-01-19T18:58:09Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/1029/
Test PASSed (JDK 8 and Scala 2.12).

asfbot · 2017-01-19T19:03:41Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/1026/
Test FAILed (JDK 8 and Scala 2.12).

asfbot · 2017-01-19T19:03:58Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/1029/
Test PASSed (JDK 7 and Scala 2.10).

xvrl · 2017-01-19T19:13:30Z

retest this please

ijuma

LGTM, assuming that someone has verified that this fixes the issue mentioned in the PR.

We should look into including Java 8 tests/examples in the codebase in the future.

asfbot · 2017-01-19T20:03:17Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/1035/
Test PASSed (JDK 8 and Scala 2.12).

asfbot · 2017-01-19T20:39:06Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/1037/
Test PASSed (JDK 8 and Scala 2.11).

asfbot · 2017-01-19T21:23:06Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/1035/
Test FAILed (JDK 7 and Scala 2.10).

xvrl · 2017-01-19T21:36:50Z

retest this please

asfbot · 2017-01-19T22:26:48Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/1039/
Test PASSed (JDK 8 and Scala 2.12).

asfbot · 2017-01-19T22:28:17Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/1041/
Test PASSed (JDK 8 and Scala 2.11).

asfbot · 2017-01-19T22:32:21Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/1039/
Test PASSed (JDK 7 and Scala 2.10).

Variance changes introduced in KIP-100 cause compilation failures with lambda expression in Java 8. To my knowledge this only affects the following method `KStreams.transform(TransformerSupplier<...>, String...)` prior to the changes it was possible to write: `streams.transform(MyTransformer::new)` where `MyTransformer` extends `Transformer` After the changes the Java compiler is unable to infer correct return types for the lambda expressions. This change fixed this by reverting to invariant return types for transformer suppliers. please cherry-pick into 0.10.2.x Author: Xavier Léauté <xavier@confluent.io> Reviewers: Ismael Juma, Damian Guy, Guozhang Wang Closes #2402 from xvrl/lambdas-oh-my (cherry picked from commit 20e957c) Signed-off-by: Guozhang Wang <wangguoz@gmail.com>

guozhangwang · 2017-01-21T22:10:34Z

LGTM, merged to trunk and 0.10.2.

Thanks @xvrl !

Variance changes introduced in KIP-100 cause compilation failures with lambda expression in Java 8. To my knowledge this only affects the following method `KStreams.transform(TransformerSupplier<...>, String...)` prior to the changes it was possible to write: `streams.transform(MyTransformer::new)` where `MyTransformer` extends `Transformer` After the changes the Java compiler is unable to infer correct return types for the lambda expressions. This change fixed this by reverting to invariant return types for transformer suppliers. please cherry-pick into 0.10.2.x Author: Xavier Léauté <xavier@confluent.io> Reviewers: Ismael Juma, Damian Guy, Guozhang Wang Closes apache#2402 from xvrl/lambdas-oh-my

dguy reviewed Jan 19, 2017

View reviewed changes

KAFKA-4672 fix source compatibility for lambda expressions

a82de26

xvrl force-pushed the lambdas-oh-my branch from 070af88 to a82de26 Compare January 19, 2017 16:46

dguy approved these changes Jan 19, 2017

View reviewed changes

ijuma approved these changes Jan 19, 2017

View reviewed changes

asfgit closed this in 20e957c Jan 21, 2017

xvrl deleted the lambdas-oh-my branch April 17, 2017 22:00

Conversation

xvrl commented Jan 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

enothereska commented Jan 18, 2017

Uh oh!

asfbot commented Jan 18, 2017

Uh oh!

xvrl commented Jan 18, 2017

Uh oh!

asfbot commented Jan 18, 2017

Uh oh!

ijuma commented Jan 18, 2017

Uh oh!

asfbot commented Jan 18, 2017

Uh oh!

xvrl commented Jan 19, 2017

Uh oh!

xvrl commented Jan 19, 2017

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

enothereska commented Jan 19, 2017

Uh oh!

dguy Jan 19, 2017

Choose a reason for hiding this comment

Uh oh!

xvrl Jan 19, 2017

Choose a reason for hiding this comment

Uh oh!

enothereska commented Jan 19, 2017

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

dguy left a comment

Choose a reason for hiding this comment

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

xvrl commented Jan 19, 2017

Uh oh!

enothereska commented Jan 19, 2017

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

xvrl commented Jan 19, 2017

Uh oh!

ijuma left a comment

Choose a reason for hiding this comment

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

xvrl commented Jan 19, 2017

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

asfbot commented Jan 19, 2017

Uh oh!

guozhangwang commented Jan 21, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

xvrl commented Jan 18, 2017 •

edited

Loading