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 > > > > > > > > > >
