Re: [DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2022-01-19 Thread Luke Chen
Hi all, If there are no other comments, I'll start to vote tomorrow. Thank you. Luke On Sat, Jan 15, 2022 at 10:47 AM Luke Chen wrote: > Hi Matthias, > > Thanks for the comments. > > 1. The config name: `default.dsl.store` looks good to me. Updated! > 2. the KIP still contains a view case of

Re: [DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2022-01-14 Thread Luke Chen
Hi Matthias, Thanks for the comments. 1. The config name: `default.dsl.store` looks good to me. Updated! 2. the KIP still contains a view case of the originally proposed config name `default.dsl.store.type` which should be updated. --> Updated. Thanks. 3. About `TopologyConfig`: we should add

Re: [DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2022-01-12 Thread Matthias J. Sax
Thanks for the KIP! I think it's a good step forward for the DSL and it makes sense to exclude the PAPI and custom stores for now. About the config name, it seems to be overly complicated. Guozhang's argument about "store type" that is used to refer to kv, windowed, session stores make

Re: [DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2021-12-22 Thread Luke Chen
Hi Guozhang, Thanks for the comments. And I agree it's better to rename it to `default.dsl.store.impl.type` for differentiation. I've updated the KIP. Thank you. Luke On Wed, Dec 22, 2021 at 3:12 AM Guozhang Wang wrote: > Thanks Luke, I do not have any major comments on the wiki any more.

Re: [DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2021-12-21 Thread Guozhang Wang
Thanks Luke, I do not have any major comments on the wiki any more. BTW thanks for making the "public StreamsBuilder(final TopologyConfig topologyConfigs)" API public now, I think it will benefit a lot of future works! I think with the new API, we can deprecate the `build(props)` function in

Re: [DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2021-12-16 Thread Luke Chen
Hi Guozhang, I've updated the KIP to use `enum`, instead of store implementation, and some content accordingly. Please let me know if there's other comments. Thank you. Luke On Sun, Dec 12, 2021 at 3:55 PM Luke Chen wrote: > Hi Guozhang, > > Thanks for your comments. > I agree that in the

Re: [DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2021-12-11 Thread Luke Chen
Hi Guozhang, Thanks for your comments. I agree that in the KIP, there's a trade-off regarding the API complexity. With the store impl, we can support default custom stores, but introduce more complexity for users, while with the enum types, users can configure default built-in store types easily,

Re: [DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2021-12-09 Thread Guozhang Wang
Thanks Luke for the updated KIP. One major change the new proposal has it to change the original enum store type with a new interface. Where in the enum approach our internal implementations would be something like: " Stores#keyValueBytesStoreSupplier(storeImplTypes, storeName, ...)

[DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2021-12-01 Thread Luke Chen
Hi devs, I'd like to propose a KIP to allow users to set default store implementation class (built-in RocksDB/InMemory, or custom one), and default to RocksDB state store, to keep backward compatibility. Detailed description can be found here: