Thanks for the KIP Leah. Should `withLoggingEnabled()` take a `Map<String, String> config` similar to the one from `Materialized`?
-Matthias On 11/30/20 12:22 PM, Walker Carlson wrote: > Ah. That makes sense. Thank you for fixing that. > > One minor question. If the default is enabled is there any case where a > user would turn logging off then back on again? I can see having the > enableLoging for completeness so it's not that important probably. > > Anyways other than that it looks good! > > Walker > > On Mon, Nov 30, 2020 at 12:06 PM Leah Thomas <ltho...@confluent.io> wrote: > >> Hey Walker, >> >> Thanks for your response. >> >> 1. Ah yeah thanks for the catch, that was held over from copy/paste. Both >> functions should take no parameters, as they just `loggingEnabled` to true >> or false. I've removed the `WindowBytesStoreSupplier otherStoreSupplier` >> from the methods in the KIP >> 2. I think the fix to 1 answers this question, otherwise, I'm not quite >> sure what you're asking. With the updated method calls, there shouldn't be >> any duplication. >> >> Cheers, >> Leah >> >> On Mon, Nov 30, 2020 at 12:14 PM Walker Carlson <wcarl...@confluent.io> >> wrote: >> >>> Hello Leah, >>> >>> Thank you for the KIP. >>> >>> I had a couple questions that maybe you can expand on from what is on the >>> KIP. >>> >>> 1) Why are we enabling/disabling the logging by passing in a >>> `WindowBytesStoreSupplier`? >>> It seems to me that these two things should be separate. >>> >>> 2) There is already `withThisStoreSupplier(final WindowBytesStoreSupplier >>> otherStoreSupplier)` and `withOtherStoreSupplier(final >>> WindowBytesStoreSupplier otherStoreSupplier)`. Why do we need to >> duplicate >>> them when the `retentionPeriod` can be set through them? >>> >>> Thanks, >>> Walker >>> >>> On Mon, Nov 30, 2020 at 8:53 AM Leah Thomas <ltho...@confluent.io> >> wrote: >>> >>>> After reading through https://issues.apache.org/jira/browse/KAFKA-9921 >> I >>>> removed the option to enable/disable caching for `StreamJoined`, as >>> caching >>>> will always be disabled because we retain duplicates. >>>> >>>> I updated the KIP accordingly, it now adds only `enableLogging` as a >>>> config. >>>> >>>> On Mon, Nov 30, 2020 at 9:54 AM Leah Thomas <ltho...@confluent.io> >>> wrote: >>>> >>>>> Hi all, >>>>> >>>>> I'd like to kick-off the discussion for KIP-689: Extend >> `StreamJoined` >>> to >>>>> allow more store configs. This builds off the work of KIP-479 >>>>> < >>>> >>> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-479%3A+Add+StreamJoined+config+object+to+Join >>>> >>>> to >>>>> add options to enable/disable both logging and caching for stream >> join >>>>> stores. >>>>> >>>>> KIP is here: >>>>> >>>> >>> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-689%3A+Extend+%60StreamJoined%60+to+allow+more+store+configs >>>>> >>>>> >>>>> Looking forward to hearing your thoughts, >>>>> Leah >>>>> >>>> >>> >> >
