Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-25 Thread Damian Guy
On Thu, 24 Aug 2017 at 18:31 Xavier Léauté wrote: > A few comments on the KIP: > - I'm a bit confused about the BytesStoreSupplier interface. Nothing in its > definition is really specific to Bytes, and > when I see return types like BytesStoreSupplier

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-25 Thread Damian Guy
Matthias, i agree so i've added those two overloads. Thanks, Damian On Thu, 24 Aug 2017 at 21:54 Matthias J. Sax wrote: > Thanks for clarification. I see your point. Java varargs are problematic > in general IMHO as they force you to put them as last argument making >

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-24 Thread Matthias J. Sax
Thanks for clarification. I see your point. Java varargs are problematic in general IMHO as they force you to put them as last argument making parameter ordering unnatural for some cases (as we have it currently in the API). Nevertheless, I think that reading a single topic is the most common

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-24 Thread Guozhang Wang
Matthias, I think it's my bad that I did not post another comment on the mailing list while syncing with Damian. Here it is: Regarding 1) above, a second thought on varargs: though I have not heard from anyone using multiple topics, it is also true that people will just keep silent until their

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-24 Thread Matthias J. Sax
We now have > public synchronized KStream stream(final Collection > topic, final Consumed options) This would prevent so write code like builder.stream("topic", Consumers.with(...)); I think, we need methods StreamsBuilder#stream(String topic); StreamsBuilder#stream(String

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-24 Thread Xavier Léauté
A few comments on the KIP: - I'm a bit confused about the BytesStoreSupplier interface. Nothing in its definition is really specific to Bytes, and when I see return types like BytesStoreSupplier>, it seems redundant to have "Bytes" in the supplier name. Why can't we

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-24 Thread Damian Guy
I've updated the kip to reflect Bill's comment and also to make StreamBuilder methods have topic as the first param, i.e., StreamBuilder#stream no longer accepts varargs. On Thu, 24 Aug 2017 at 09:12 Damian Guy wrote: > On Thu, 24 Aug 2017 at 02:49 Guozhang Wang

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-24 Thread Damian Guy
On Thu, 24 Aug 2017 at 02:49 Guozhang Wang wrote: > I have a couple of comments but otherwise it LGTM: > > 1. For these two functions in StreamsBuilder, the topic String is set as > the second parameter in between of two options. Would that be better to be > set as the first

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-23 Thread Guozhang Wang
I have a couple of comments but otherwise it LGTM: 1. For these two functions in StreamsBuilder, the topic String is set as the second parameter in between of two options. Would that be better to be set as the first or the last one instead? public synchronized KTable table(final

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-23 Thread Damian Guy
We already have GlobalKTable and i can't rename KGroupedStream, which really should be GroupedKStream. So I think we should name new things correctly, i.e., WindowedKStream etc and fix the others when we can. On Wed, 23 Aug 2017 at 20:38 Matthias J. Sax wrote: > About

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-23 Thread Damian Guy
Thanks Bill On Wed, 23 Aug 2017 at 20:24 Bill Bejeck wrote: > Thanks for all the work on this KIP Damian. > > Both `Produced` and `Joined` have a `with` method accepting all parameters, > but `Consumed` doesn't. Should we add one for consistency? > > Yep, i'll update it >

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-23 Thread Matthias J. Sax
About KGroupedStream vs GroupedKStream: shouldn't we keep the naming convention consistent? And if we change the naming schema just change all at once? I personally don't care which naming scheme is better, but I think consistency is super important! About Bill's comment: I agree, and had a

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-23 Thread Bill Bejeck
Thanks for all the work on this KIP Damian. Both `Produced` and `Joined` have a `with` method accepting all parameters, but `Consumed` doesn't. Should we add one for consistency? Thanks, Bill On Wed, Aug 23, 2017 at 4:15 AM, Damian Guy wrote: > KIP has been updated.

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-23 Thread Damian Guy
KIP has been updated. thanks On Wed, 23 Aug 2017 at 09:10 Damian Guy wrote: > Hi Matthias, > > >> KStream: >> leftJoin and outerJoin for KStream/KTable join should not have >> `JoinWindows` parameter >> >> Thanks! > > >> >> Nit: TopologyBuilder -> Topology >> >> Ack > > >>

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-23 Thread Damian Guy
Hi Matthias, > KStream: > leftJoin and outerJoin for KStream/KTable join should not have > `JoinWindows` parameter > > Thanks! > > Nit: TopologyBuilder -> Topology > > Ack > Nit: new class Serialized list static method #with twice > > Ack > WindowedKStream -> for consistency we should

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-22 Thread Matthias J. Sax
Thanks to the update Damian! Couple of comments: KStream: leftJoin and outerJoin for KStream/KTable join should not have `JoinWindows` parameter Nit: TopologyBuilder -> Topology Nit: new class Serialized list static method #with twice WindowedKStream -> for consistency we should either

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-22 Thread Damian Guy
I've just updated the KIP with some additional changes targeted at StreamsBuilder Thanks, Damian On Thu, 10 Aug 2017 at 12:59 Damian Guy wrote: > >> Got it, thanks. >> >> Does it still make sense to have one static constructors for each spec, >> with one constructor

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-10 Thread Damian Guy
> Got it, thanks. > > Does it still make sense to have one static constructors for each spec, > with one constructor having only one parameter to make it more usable, i.e. > as a user I do not need to give all parameters if I only want to override > one of them? Maybe we can just name the

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-09 Thread Damian Guy
On Wed, 9 Aug 2017 at 20:00 Guozhang Wang wrote: > >> Another comment about Printed in general is it differs with other > options > >> that it is a required option than optional one, since it includes > toSysOut > >> / toFile specs; what are the pros and cons for including

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-09 Thread Guozhang Wang
>> Another comment about Printed in general is it differs with other options >> that it is a required option than optional one, since it includes toSysOut >> / toFile specs; what are the pros and cons for including these two in the >> option and hence make it a required option than leaving them at

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-09 Thread Guozhang Wang
>> The key idea is that by using the same function name string for static >> constructor and member functions, users do not need to remember what are >> the differences but can call these functions with any ordering they want, >> and later calls on the same spec will win over early calls. >> >>

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-09 Thread Damian Guy
On Tue, 8 Aug 2017 at 20:11 Guozhang Wang wrote: > Damian, > > Thanks for the proposal, I had a few comments on the APIs: > > 1. Printed#withFile seems not needed, as users should always spec if it is > to sysOut or to File at the beginning. In addition as a second thought, I

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-08-08 Thread Guozhang Wang
Damian, Thanks for the proposal, I had a few comments on the APIs: 1. Printed#withFile seems not needed, as users should always spec if it is to sysOut or to File at the beginning. In addition as a second thought, I think serdes are not useful for prints anyways since we assume `toString` is

Re: [DISCUSS] KIP-182: Reduce Streams DSL overloads and allow easier use of custom storage engines

2017-07-27 Thread Damian Guy
Updated link: https://cwiki.apache.org/confluence/display/KAFKA/KIP-182%3A+Reduce+Streams+DSL+overloads+and+allow+easier+use+of+custom+storage+engines Thanks, Damian On Thu, 27 Jul 2017 at 13:09 Damian Guy wrote: > Hi, > > I've put together a KIP to make some changes to