Hi Debasish, You can work on the PR in parallel to the KIP discussion so that people can start reviewing it. The only restriction is that the PR cannot be merged until the KIP is accepted.
Guozhang On Mon, Mar 19, 2018 at 11:09 AM, Debasish Ghosh < debasish.gh...@lightbend.com> wrote: > Based on this discussion I have a question .. > > For the actual implementation we need to have a PR on > https://github.com/apache/kafka and the new library will be in the package > structure org.apache.kafka.streams.scala. My question is, is this PR part > of the KIP ? Or we work on the PR once the KIP is accepted .. > > regards. > > On Mon, Mar 19, 2018 at 11:28 PM, Debasish Ghosh < > debasish.gh...@lightbend.com> wrote: > > > Thanks Guozhang for the comments .. we will start working on them .. > > > > regards. > > > > On Mon, Mar 19, 2018 at 11:02 PM, Guozhang Wang <wangg...@gmail.com> > > wrote: > > > >> And one more comment about the type safety: > >> > >> 7. I'm in favor of the approach of enforcing type safety at compile > time, > >> since with Scala's strong typing system I think it makes more sense to > get > >> rid of config-based serde specifications that can cause runtime error in > >> the Scala wrapper. > >> > >> > >> Guozhang > >> > >> > >> On Mon, Mar 19, 2018 at 10:28 AM, Guozhang Wang <wangg...@gmail.com> > >> wrote: > >> > >> > Hello Debasish, > >> > > >> > Thanks for the KIP. Here are some comments: > >> > > >> > 1. About the naming: I'd also vote for option 2 and enforce users to > >> > rename when import since this scenario seems rare to me. > >> > > >> > 2. About the dependency: since this KIP can only be merged as early as > >> > 1.2, it means that for users who wants to use this artifact, say for > >> > version 1.2, she would need to bring in "kafka-streams" version 1.2. > In > >> > addition, if we change the Java API in the future versions we'd also > >> need > >> > to update the Scala wrapper at the same version as well, so in other > >> words > >> > "kafka-streams-scala" version X have to be with "kafka-streams" > version > >> X > >> > anyways. So I'd suggest we remove the version number in > >> "kafka-streams-scala > >> > only depends on the Scala standard library and Kafka Streams" as it > may > >> > be confusing to readers. > >> > > >> > 3. For the KIP discussion, it need be based on the proposed state of > >> this > >> > project in AK. More specifically, for the groupId in maven / gradle, > it > >> > need to be "org.apache.kafka"; for the version, it need to be Kafka > >> release > >> > versions, e.g. 1.2.0. > >> > > >> > 4. In "New or Changed Public Interfaces" section, it does not mention > >> the > >> > serde classes like "DefaultSerdes", but I think they should also be > >> > considered public APIs as users would most likely import these in her > >> > implementation. > >> > > >> > 5. Could you also list the changes you'd propose to made to > build.gradle > >> > in kafka for adding this artifact? More details will help readers to > >> better > >> > understand your proposal. > >> > > >> > 6. I think it'd will be good to have a WordCount example code as part > of > >> > this KIP, to illustrate how to code in this Scala wrapper, e.g. in > >> > o.a.k.scala.examples. But for this class we probably do not need to > >> have a > >> > separate artifact for it as we did in kafka-streams-examples. > >> > > >> > > >> > Guozhang > >> > > >> > > >> > On Mon, Mar 19, 2018 at 9:16 AM, Ted Yu <yuzhih...@gmail.com> wrote: > >> > > >> >> I agree with Sean on name unification. > >> >> > >> >> +1 to option 2. > >> >> > >> >> On Mon, Mar 19, 2018 at 7:23 AM, Sean Glover < > >> sean.glo...@lightbend.com> > >> >> wrote: > >> >> > >> >> > Type names: I vote for option 2. The user must explicitly add a > >> >> dependency > >> >> > to this library and the wrapper types are in a different package. > It > >> >> seems > >> >> > reasonable to expect them to do an import rename if there's a need > to > >> >> drop > >> >> > down to the Java API. > >> >> > > >> >> > Test Utils: The test utils in kafka-streams-scala are nice and > lean, > >> but > >> >> > I'm not sure if it provides much more value than other options that > >> >> exist > >> >> > in the community. There's an embedded Kafka/ZK project > >> implementation > >> >> for > >> >> > ScalaTest that's popular and active: manub/scalatest-embedded- > kakfa. > >> >> It > >> >> > implies you must also use ScalaTest, which I acknowledge isn't > >> >> everyone's > >> >> > first choice for Scala test framework, but it probably is one of, > if > >> not > >> >> > the most, popular library. It includes a DSL for Kafka Streams > >> too. If > >> >> > this KIP is accepted then perhaps a PR to that project could be > made > >> to > >> >> > support the new wrapper implementations. > >> >> > > >> >> > https://github.com/manub/scalatest-embedded-kafka# > >> >> > scalatest-embedded-kafka-streams > >> >> > > >> >> > Sean > >> >> > > >> >> > On Sun, Mar 18, 2018 at 4:05 AM, Debasish Ghosh < > >> >> > debasish.gh...@lightbend.com> wrote: > >> >> > > >> >> > > > > >> >> > > > > Should this be 1.2 (maybe it's even better to not put any > >> >> version at > >> >> > > > all) > >> >> > > > >> >> > > > >> >> > > Actually wanted to emphasize that the support is from 1.0.0 > >> onwards .. > >> >> > > Should we make that explicit ? Like .. > >> >> > > > >> >> > > kafka-streams-scala only depends on the Scala standard library > and > >> >> Kafka > >> >> > > > Streams 1.0.0+. > >> >> > > > >> >> > > > >> >> > > In 1.1 release, we add a new module `kafka-streams-test-utils` > to > >> >> > simplify > >> >> > > > testing for Kafka Streams applications. Are those test utils > >> >> suitable > >> >> > for > >> >> > > > Scala users or should we add Scala wrappers for those, too? > >> >> > > > >> >> > > > >> >> > > I will check up and let you know .. > >> >> > > > >> >> > > Also I am not clear about the decision on renaming of Scala > >> >> abstractions. > >> >> > > Can we have a consensus on this ? Here's the summary .. > >> >> > > > >> >> > > *Option 1:* Keep names separate (KStream for Java class, KStreamS > >> for > >> >> > > Scala). No renaming of imports required. > >> >> > > *Option 2:* Unify names (KStream for Java and Scala class names). > >> No > >> >> > > conflict since they will reside in different packages. But if we > >> need > >> >> to > >> >> > > use both abstractions, renaming of imports are required. But > again, > >> >> this > >> >> > > may not be a too frequent use case. > >> >> > > > >> >> > > Suggestions ? > >> >> > > > >> >> > > regards. > >> >> > > > >> >> > > On Sat, Mar 17, 2018 at 3:07 AM, Matthias J. Sax < > >> >> matth...@confluent.io> > >> >> > > wrote: > >> >> > > > >> >> > > > Thanks a lot for the KIP! Two questions: > >> >> > > > > >> >> > > > 1) the KIP states: > >> >> > > > > >> >> > > > > kafka-streams-scala only depends on the Scala standard > library > >> and > >> >> > > Kafka > >> >> > > > Streams 1.0.0. > >> >> > > > > >> >> > > > Should this be 1.2 (maybe it's even better to not put any > >> version > >> >> at > >> >> > > all) > >> >> > > > > >> >> > > > > >> >> > > > 2) In 1.1 release, we add a new module > >> `kafka-streams-test-utils` to > >> >> > > > simplify testing for Kafka Streams applications. Are those test > >> >> utils > >> >> > > > suitable for Scala users or should we add Scala wrappers for > >> those, > >> >> > too? > >> >> > > > > >> >> > > > > >> >> > > > -Matthias > >> >> > > > > >> >> > > > > >> >> > > > On 3/16/18 11:10 AM, Ted Yu wrote: > >> >> > > > > Import renames seem to be fine. > >> >> > > > > > >> >> > > > > The class names with trailing 'S' look clean. > >> >> > > > > > >> >> > > > > Cheers > >> >> > > > > > >> >> > > > > On Fri, Mar 16, 2018 at 11:04 AM, Ismael Juma < > >> ism...@juma.me.uk> > >> >> > > wrote: > >> >> > > > > > >> >> > > > >> If this is rare (as it sounds), relying on import renames > >> seems > >> >> fine > >> >> > > to > >> >> > > > me. > >> >> > > > >> Let's see what others think. > >> >> > > > >> > >> >> > > > >> Ismael > >> >> > > > >> > >> >> > > > >> On Fri, Mar 16, 2018 at 10:51 AM, Debasish Ghosh < > >> >> > > > >> debasish.gh...@lightbend.com> wrote: > >> >> > > > >> > >> >> > > > >>> I am not sure if this is practical or not. But > theoretically > >> a > >> >> user > >> >> > > may > >> >> > > > >>> want to extract the unsafe Java abstraction from the Scala > >> ones > >> >> and > >> >> > > use > >> >> > > > >>> Java APIs on them .. e.g. > >> >> > > > >>> > >> >> > > > >>> val userClicksStream: KStreamS[String, Long] = > >> >> > > > >>> builder.stream(userClicksTopic) // Scala abstraction > >> >> > > > >>> > >> >> > > > >>> val jStream: KStream[String, Long] = userClicksStream.inner > >> // > >> >> > > > publishes > >> >> > > > >>> the underlying Java abstraction > >> >> > > > >>> > >> >> > > > >>> //.. work with Java, may be pass to some function written > in > >> >> Java > >> >> > > > >>> > >> >> > > > >>> I do realize this is somewhat of a convoluted use case and > >> may > >> >> not > >> >> > be > >> >> > > > >>> practically useful .. > >> >> > > > >>> > >> >> > > > >>> Otherwise we can very well work on the suggested approach > of > >> >> > unifying > >> >> > > > the > >> >> > > > >>> names .. > >> >> > > > >>> > >> >> > > > >>> regards. > >> >> > > > >>> > >> >> > > > >>> > >> >> > > > >>> > >> >> > > > >>> On Fri, Mar 16, 2018 at 10:28 PM, Ismael Juma < > >> >> ism...@juma.me.uk> > >> >> > > > wrote: > >> >> > > > >>> > >> >> > > > >>>> What does "mixed mode application" mean? What are the > cases > >> >> where > >> >> > a > >> >> > > > >> user > >> >> > > > >>>> would want to use both APIs? I think that would help > >> understand > >> >> > the > >> >> > > > >>>> reasoning. > >> >> > > > >>>> > >> >> > > > >>>> Thanks, > >> >> > > > >>>> Ismael > >> >> > > > >>>> > >> >> > > > >>>> On Fri, Mar 16, 2018 at 8:48 AM, Debasish Ghosh < > >> >> > > > >>>> debasish.gh...@lightbend.com> wrote: > >> >> > > > >>>> > >> >> > > > >>>>> Hi Damian - > >> >> > > > >>>>> > >> >> > > > >>>>> We could. But in case the user wants to use both Scala > and > >> >> Java > >> >> > > APIs > >> >> > > > >>> (may > >> >> > > > >>>>> be for some mixed mode application), won't that be > >> confusing ? > >> >> > She > >> >> > > > >> will > >> >> > > > >>>>> have to do something like .. > >> >> > > > >>>>> > >> >> > > > >>>>> import o.a.k.s.scala.{KStream => KStreamS} > >> >> > > > >>>>> > >> >> > > > >>>>> to rename Scala imports or the other way round for > imported > >> >> Java > >> >> > > > >>> classes. > >> >> > > > >>>>> > >> >> > > > >>>>> regards. > >> >> > > > >>>>> > >> >> > > > >>>>> > >> >> > > > >>>>> > >> >> > > > >>>>> On Fri, Mar 16, 2018 at 9:07 PM, Damian Guy < > >> >> > damian....@gmail.com> > >> >> > > > >>>> wrote: > >> >> > > > >>>>> > >> >> > > > >>>>>> Hi Debasish, > >> >> > > > >>>>>> > >> >> > > > >>>>>> Thanks for the KIP - will be a great addition to > streams. > >> >> I've > >> >> > > only > >> >> > > > >>>> had a > >> >> > > > >>>>>> quick scan, but seeing as the Scala classes are going to > >> be > >> >> in > >> >> > > > >> their > >> >> > > > >>>> own > >> >> > > > >>>>>> package could we drop the S at the end of the class > names? > >> >> > > > >>>>>> > >> >> > > > >>>>>> Thanks, > >> >> > > > >>>>>> Damian > >> >> > > > >>>>>> > >> >> > > > >>>>>> > >> >> > > > >>>>>> On Fri, 16 Mar 2018 at 15:25 Debasish Ghosh < > >> >> > > > >>>>> debasish.gh...@lightbend.com> > >> >> > > > >>>>>> wrote: > >> >> > > > >>>>>> > >> >> > > > >>>>>>> Hi - > >> >> > > > >>>>>>> > >> >> > > > >>>>>>> A new KIP, KIP-270 is up for discussion: > >> >> > > > >>>>>>> > >> >> > > > >>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- > >> >> > > > >>>>>> 270+-+A+Scala+Wrapper+Library+for+Kafka+Streams > >> >> > > > >>>>>>> > >> >> > > > >>>>>>> The relevant JIRA issue: https://issues.apache.org/ > >> >> > > > >>>>>> jira/browse/KAFKA-6670 > >> >> > > > >>>>>>> > >> >> > > > >>>>>>> The library as proposed in the KIP has been implemented > >> at > >> >> > > > >>>>>>> https://github.com/lightbend/kafka-streams-scala and > the > >> >> > current > >> >> > > > >>>>> release > >> >> > > > >>>>>>> is > >> >> > > > >>>>>>> 0.2.0 ( > >> >> > > > >>>>>>> https://github.com/lightbend/k > >> afka-streams-scala/releases/ > >> >> > > > >>> tag/v0.2.0 > >> >> > > > >>>> ). > >> >> > > > >>>>>>> We at Lightbend has been using it since quite some time > >> now. > >> >> > > > >>>>>>> > >> >> > > > >>>>>>> regards. > >> >> > > > >>>>>>> > >> >> > > > >>>>>>> -- > >> >> > > > >>>>>>> Debasish Ghosh > >> >> > > > >>>>>>> Principal Engineer > >> >> > > > >>>>>>> > >> >> > > > >>>>>>> Twitter: @debasishg > >> >> > > > >>>>>>> Blog: http://debasishg.blogspot.com > >> >> > > > >>>>>>> Code: https://github.com/debasishg > >> >> > > > >>>>>>> > >> >> > > > >>>>>> > >> >> > > > >>>>> > >> >> > > > >>>>> > >> >> > > > >>>>> > >> >> > > > >>>>> -- > >> >> > > > >>>>> Debasish Ghosh > >> >> > > > >>>>> Principal Engineer > >> >> > > > >>>>> > >> >> > > > >>>>> Twitter: @debasishg > >> >> > > > >>>>> Blog: http://debasishg.blogspot.com > >> >> > > > >>>>> Code: https://github.com/debasishg > >> >> > > > >>>>> > >> >> > > > >>>> > >> >> > > > >>> > >> >> > > > >>> > >> >> > > > >>> > >> >> > > > >>> -- > >> >> > > > >>> Debasish Ghosh > >> >> > > > >>> Principal Engineer > >> >> > > > >>> > >> >> > > > >>> Twitter: @debasishg > >> >> > > > >>> Blog: http://debasishg.blogspot.com > >> >> > > > >>> Code: https://github.com/debasishg > >> >> > > > >>> > >> >> > > > >> > >> >> > > > > > >> >> > > > > >> >> > > > > >> >> > > > >> >> > > > >> >> > > -- > >> >> > > Debasish Ghosh > >> >> > > Principal Engineer > >> >> > > > >> >> > > Twitter: @debasishg > >> >> > > Blog: http://debasishg.blogspot.com > >> >> > > Code: https://github.com/debasishg > >> >> > > > >> >> > > >> >> > > >> >> > > >> >> > -- > >> >> > Senior Software Engineer, Lightbend, Inc. > >> >> > > >> >> > <http://lightbend.com> > >> >> > > >> >> > @seg1o <https://twitter.com/seg1o>, in/seanaglover > >> >> > <https://www.linkedin.com/in/seanaglover/> > >> >> > > >> >> > >> > > >> > > >> > > >> > -- > >> > -- Guozhang > >> > > >> > >> > >> > >> -- > >> -- Guozhang > >> > > > > > > > > -- > > Debasish Ghosh > > Principal Engineer > > > > Twitter: @debasishg > > Blog: http://debasishg.blogspot.com > > Code: https://github.com/debasishg > > > > > > > > > > > -- > Debasish Ghosh > Principal Engineer > > Twitter: @debasishg > Blog: http://debasishg.blogspot.com > Code: https://github.com/debasishg > -- -- Guozhang
